From 0ad60eabe065da835c3ed8141bdaf902b337f0c8 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Thu, 29 Sep 2022 13:44:41 -0700 Subject: [PATCH] Update to latest git, backport PR #2177 to clean up video devices --- .gitignore | 1 + ...-video-device-setting-deprecate-QEMU.patch | 222 ++++++++++++++++++ os-autoinst.spec | 15 +- sources | 2 +- 4 files changed, 236 insertions(+), 4 deletions(-) create mode 100644 0001-Consolidate-qemu-video-device-setting-deprecate-QEMU.patch diff --git a/.gitignore b/.gitignore index 503a193..442af0c 100644 --- a/.gitignore +++ b/.gitignore @@ -56,3 +56,4 @@ /os-autoinst-8a7a14fa5175e3ce1dbd0f59c2c2ec2d73c572d6.tar.gz /os-autoinst-ddf414b8d83576d0f5373c15acd70ab2c3ea9fb8.tar.gz /os-autoinst-d3d433bda958b473ba7c60bb434760566c0d356b.tar.gz +/os-autoinst-436f134262416559c5b7248d4246cbfed67ae835.tar.gz diff --git a/0001-Consolidate-qemu-video-device-setting-deprecate-QEMU.patch b/0001-Consolidate-qemu-video-device-setting-deprecate-QEMU.patch new file mode 100644 index 0000000..4bd7648 --- /dev/null +++ b/0001-Consolidate-qemu-video-device-setting-deprecate-QEMU.patch @@ -0,0 +1,222 @@ +From 02f80e36cf982c2397cb6efc347106b023bebf75 Mon Sep 17 00:00:00 2001 +From: Adam Williamson +Date: Tue, 13 Sep 2022 11:09:15 -0700 +Subject: [PATCH] Consolidate qemu video device setting, deprecate QEMUVGA + +As discussed in #2170, this code has progressively become a bit +of a mess as special cases were added for different arches and +things changed in qemu. This rewrites it to at least consolidate +the mess in one place, and deprecate some things which will +allow us to simplify it in future. + +Always setting `-device` rather than sometimes `-vga` and +sometimes `-device` allows us to be much more consistent. We +'translate' `QEMUVGA` values into `-device` values after warning +that it is deprecated. Eventually we can hopefully get rid of +the handling of the deprecated vars, and the code will then be +much clearer and simpler. + +References for this: +https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/ +https://www.kraxel.org/blog/2019/03/edid-support-for-qemu/ + +Signed-off-by: Adam Williamson +--- + backend/qemu.pm | 59 ++++++++++++++++++++++++++------------- + doc/backend_vars.asciidoc | 11 ++++---- + t/18-backend-qemu.t | 29 +++++++++++++++---- + 3 files changed, 68 insertions(+), 31 deletions(-) + +diff --git a/backend/qemu.pm b/backend/qemu.pm +index 36124879..35f2fb32 100644 +--- a/backend/qemu.pm ++++ b/backend/qemu.pm +@@ -501,7 +501,6 @@ sub delete_virtio_console_fifo () { unlink $_ or bmwqemu::fctwarn("Could not unl + + sub qemu_params_ofw ($self) { + my $vars = \%bmwqemu::vars; +- $vars->{QEMUVGA} ||= "std"; + $vars->{QEMUMACHINE} //= "usb=off"; + # set the initial resolution on PCC and SPARC + sp('g', "$self->{xres}x$self->{yres}"); +@@ -547,18 +546,38 @@ sub setup_tpm ($self, $arch) { + } + } + +-sub _set_graphics_backend ($self) { +- # note: Specifying EDID information explicitly for std/virtio backends to get consistent behavior across different QEMU +- # versions (as of QEMU 7.0.0 the default resolution is no longer 1024x768). +- +- my $qemu_vga = $bmwqemu::vars{QEMUVGA}; +- if (!$qemu_vga || $qemu_vga eq 'std') { +- sp('device', "VGA,edid=on,xres=$self->{xres},yres=$self->{yres}"); +- } elsif ($qemu_vga eq 'virtio') { +- sp('device', "virtio-vga,edid=on,xres=$self->{xres},yres=$self->{yres}"); +- } else { +- sp('vga', $qemu_vga); # adding this only if not specifying a device; otherwise we'd end up with two graphic cards ++sub _set_graphics_backend ($self, $is_arm) { ++ my $vars = \%bmwqemu::vars; ++ my $device = "VGA"; ++ my $options = ""; ++ if ($vars->{QEMU_OVERRIDE_VIDEO_DEVICE_AARCH64}) { ++ bmwqemu::fctwarn("QEMU_OVERRIDE_VIDEO_DEVICE_AARCH64 is deprecated, please set QEMU_VIDEO_DEVICE=VGA instead"); ++ } ++ else { ++ # annoying pre-existing special-case default for ARM ++ $device = "virtio-gpu-pci" if ($is_arm); ++ } ++ if ($vars->{QEMU_VIDEO_DEVICE}) { ++ bmwqemu::fctwarn("Both QEMUVGA and QEMU_VIDEO_DEVICE set, ignoring deprecated QEMUVGA!") if $vars->{QEMUVGA}; ++ $device = $vars->{QEMU_VIDEO_DEVICE}; ++ } ++ elsif ($vars->{QEMUVGA}) { ++ my $vga = $vars->{QEMUVGA}; ++ bmwqemu::fctwarn("QEMUVGA is deprecated, please set QEMU_VIDEO_DEVICE"); ++ $device = "virtio-vga" if ($vga eq "virtio"); ++ $device = "qxl-vga" if ($vga eq "qxl"); ++ $device = "cirrus-vga" if ($vga eq "cirrus"); ++ $device = "VGA" if ($vga eq "std"); ++ } ++ my @edids = ("VGA", "virtio-vga", "virtio-gpu-pci", "bochs-display"); ++ if (grep { $device eq $_ } @edids) { ++ # these devices support EDID ++ $options = ",edid=on,xres=$self->{xres},yres=$self->{yres}"; ++ } ++ if ($vars->{QEMU_VIDEO_DEVICE_OPTIONS}) { ++ $options .= "," . $vars->{QEMU_VIDEO_DEVICE_OPTIONS}; + } ++ sp('device', "${device}${options}"); + } + + sub start_qemu ($self) { +@@ -678,24 +697,24 @@ sub start_qemu ($self) { + $vars->{VDE_PORT} ||= ($vars->{WORKER_ID} // 0) * 2 + 2; + } + +- # misc +- my $arch_supports_boot_order = $vars->{UEFI} ? 0 : 1; # UEFI/OVMF supports ",bootindex=N", but not "-boot order=X" +- my $use_usb_kbd; ++ # arch discovery + my $arch = $vars->{ARCH} // ''; + $arch = 'arm' if ($arch =~ /armv6|armv7/); + my $is_arm = $arch eq 'aarch64' || $arch eq 'arm'; +- my $custom_video_device = $vars->{QEMU_VIDEO_DEVICE} // 'virtio-gpu-pci'; ++ ++ $self->_set_graphics_backend($is_arm); ++ ++ # misc ++ my $arch_supports_boot_order = $vars->{UEFI} ? 0 : 1; # UEFI/OVMF supports ",bootindex=N", but not "-boot order=X" ++ my $use_usb_kbd; + + if ($is_arm) { +- my $video_device = ($vars->{QEMU_OVERRIDE_VIDEO_DEVICE_AARCH64}) ? 'VGA' : "${custom_video_device},xres=$self->{xres},yres=$self->{yres}"; +- sp('device', $video_device); + $arch_supports_boot_order = 0; + $use_usb_kbd = 1; + } + elsif ($vars->{OFW}) { + $use_usb_kbd = $self->qemu_params_ofw; + } +- $self->_set_graphics_backend unless $is_arm; + + my @nicmac; + my @nicvlan; +@@ -798,7 +817,7 @@ sub start_qemu ($self) { + } + { + # Remove floppy drive device on architectures +- sp('global', 'isa-fdc.fdtypeA=none') unless ($arch eq 'aarch64' || $arch eq 'arm' || $vars->{QEMU_NO_FDC_SET}); ++ sp('global', 'isa-fdc.fdtypeA=none') unless ($is_arm || $vars->{QEMU_NO_FDC_SET}); + + sp('m', $vars->{QEMURAM}) if $vars->{QEMURAM}; + sp('machine', $vars->{QEMUMACHINE}) if $vars->{QEMUMACHINE}; +diff --git a/doc/backend_vars.asciidoc b/doc/backend_vars.asciidoc +index b5854918..23766252 100644 +--- a/doc/backend_vars.asciidoc ++++ b/doc/backend_vars.asciidoc +@@ -44,8 +44,8 @@ UPLOAD_METER;boolean;0;Display curl progress meter in `upload_logs()` and `uploa + UPLOAD_MAX_MESSAGE_SIZE_GB;integer;0;Specifies the max. upload size in GiB for the test API functions `upload_logs()` and `upload_assets()` and the underlying command server API. Zero denotes infinity. + UPLOAD_INACTIVITY_TIMEOUT;integer;300;Specifies the inactivity timeout in seconds for the test API functions `upload_logs()` and `upload_assets()` and underlying the command server API. + NO_DEPRECATE_BACKEND_$backend;boolean;0;Only warn about deprecated backends instead of aborting +-XRES;integer;1024;Resolution of display on x axis +-YRES;integer;768;Resolution of display on y axis ++XRES;integer;1024;Resolution of display on x axis. Sets the resolution of the video encoder, and in qemu, the initial console resolution when OFW is set (Power and SPARC), and the EDID resolution for devices that support EDID ++YRES;integer;768;Resolution of display on y axis. Sets the resolution of the video encoder, and in qemu, the initial console resolution when OFW is set (Power and SPARC), and the EDID resolution for devices that support EDID + + |==================== + +@@ -129,7 +129,7 @@ OFW;boolean;0;QEMU Open Firmware is in use + OVS_DEBUG;integer;undef;Set debug mode if value is 1 + QEMU_ONLY_EXEC;boolean;undef;If set, only execute the qemu process but return early before connecting to the process. This can be helpful for cutting testing time or to connect to the qemu process manually. + QEMU_WAIT_FINISH;boolean;undef;Only used for internal testing, see comment in t/18-qemu-options.t for details. +-QEMU_OVERRIDE_VIDEO_DEVICE_AARCH64;boolean;undef;If set, for aarch64 systems use VGA as video adapter ++QEMU_OVERRIDE_VIDEO_DEVICE_AARCH64;boolean;undef;(Deprecated, set QEMU_VIDEO_DEVICE=VGA) If set, and QEMU_VIDEO_DEVICE is not set, for arm systems use VGA as video adapter instead of virtio-gpu-pci + QEMU_DISABLE_SNAPSHOTS;boolean;undef;If set, disable snapshots in QEMU. This needs to be set when using vmdk disk images or in case the worker has slow disks to avoid save_vm calls failing due to timeouts (See https://bugzilla.suse.com/show_bug.cgi?id=1035453[bsc#1035453]) + QEMU_QMP_CONNECT_ATTEMPTS;integer;20;The number of attempts to connect to qemu qmp. Usually used for internal testing + PATHCNT;integer;2;Number of paths in MULTIPATH scenario +@@ -148,7 +148,7 @@ QEMUTHREADS;integer;undef;Number of CPU threads used by VM + QEMUTPM;'instance' or appropriate value for local swtpm config;undef;Configure VM to use a TPM emulator device, with appropriate args for the arch. If a TPM device is available at QEMUTPM_PATH_PREFIX + X, where X is the value of QEMUTPM or the worker instance number if QEMUTPM is set to 'instance', it will be used. Otherwise it will be created at test startup. See QEMUTPM_VER in the latter case. + QEMUTPM_VER;'1.2' or '2.0' depending on which TPM chip should be emulated;'2.0';If no TPM device has been setup otherwise, swtpm will be used internally to create one with a socket at /tmp/mytpmX + QEMUTPM_PATH_PREFIX;string;'/tmp/mytpm';Path prefix to use or create TPM emulator device in +-QEMUVGA;see qemu -device ?;QEMU's default;VGA device to use with VM ++QEMUVGA;virtio,qxl,cirrus,std;See QEMU_VIDEO_DEVICE;(Deprecated, use QEMU_VIDEO_DEVICE instead) VGA device to use with VM (will be converted to a matching -device parameter) + QEMU_COMPRESS_QCOW2;boolean;1;compress qcow2 images intended for upload + QEMU_IMG_CREATE_TRIES;integer;3;Define number of tries for qemu-img commands + QEMU_HUGE_PAGES_PATH;string;undef;Define a path to use huge pages (e.g. /dev/hugepages/) +@@ -195,7 +195,8 @@ VNC;integer;worker instance + 90;Display on which VNC server is running. Actual + VNCKB;;;Set the keyboard layout if you are not using en-us + WORKER_CLASS;string;undef;qemu system types + WORKER_HOSTNAME;string;undef;Worker hostname +-QEMU_VIDEO_DEVICE;string;virtio-gpu-pci;Video device to use for ARM architectures (can have options appended e.g. virtio-gpu-gl,edid=on) ++QEMU_VIDEO_DEVICE;string;virtio-gpu-pci on ARM, VGA otherwise;Video device to use with VM (using -device, not -vga). Can have options appended e.g. "virtio-gpu-gl,edid=on", but it is better to set QEMU_VIDEO_DEVICE_OPTIONS. See qemu docs and https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/ for valid choices ++QEMU_VIDEO_DEVICE_OPTIONS;string;none;Additional options for QEMU_VIDEO_DEVICE (comma-separated). Will be appended after automatically-generated resolution setting options on devices that support EDID + |==================== + + .SVIRT backend +diff --git a/t/18-backend-qemu.t b/t/18-backend-qemu.t +index 854649ca..41f1cebf 100755 +--- a/t/18-backend-qemu.t ++++ b/t/18-backend-qemu.t +@@ -134,16 +134,33 @@ subtest 'switch_network' => sub { + subtest 'setting graphics backend' => sub { + my @params; + local *backend::qemu::sp = sub (@args) { @params = @args }; +- $backend->_set_graphics_backend; +- is_deeply \@params, [device => 'VGA,edid=on,xres=1024,yres=768'], 'consistent EDID info set for std backend (no QEMUVGA set)' or diag explain \@params; ++ $backend->_set_graphics_backend(0); ++ is_deeply \@params, [device => 'VGA,edid=on,xres=1024,yres=768'], 'default backend is VGA with EDID info (no QEMUVGA or QEMU_VIDEO_DEVICE set)' or diag explain \@params; ++ ++ $backend->_set_graphics_backend(1); ++ is_deeply \@params, [device => 'virtio-gpu-pci,edid=on,xres=1024,yres=768'], 'default backend for ARM is virtio with EDID info (no QEMUVGA or QEMU_VIDEO_DEVICE set)' or diag explain \@params; ++ ++ $bmwqemu::vars{QEMU_OVERRIDE_VIDEO_DEVICE_AARCH64} = '1'; ++ $backend->_set_graphics_backend(1); ++ is_deeply \@params, [device => 'VGA,edid=on,xres=1024,yres=768'], 'QEMU_OVERRIDE_VIDEO_DEVICE_AARCH64 changes ARM default to VGA' or diag explain \@params; ++ delete $bmwqemu::vars{QEMU_OVERRIDE_VIDEO_DEVICE_AARCH64}; + + $bmwqemu::vars{QEMUVGA} = 'virtio'; +- $backend->_set_graphics_backend; +- is_deeply \@params, [device => 'virtio-vga,edid=on,xres=1024,yres=768'], 'consistent EDID info set for virtio backend' or diag explain \@params; ++ $backend->_set_graphics_backend(0); ++ is_deeply \@params, [device => 'virtio-vga,edid=on,xres=1024,yres=768'], 'QEMUVGA=virtio results in device virtio-vga with EDID info' or diag explain \@params; + + $bmwqemu::vars{QEMUVGA} = 'cirrus'; +- $backend->_set_graphics_backend; +- is_deeply \@params, [vga => 'cirrus'], 'other backends passes as-is via "-vga" parameter' or diag explain \@params; ++ $backend->_set_graphics_backend(0); ++ is_deeply \@params, [device => 'cirrus-vga'], 'QEMUVGA=cirrus results in device cirrus-vga with no EDID info' or diag explain \@params; ++ ++ $bmwqemu::vars{QEMU_VIDEO_DEVICE} = 'virtio-vga'; ++ $backend->_set_graphics_backend(0); ++ is_deeply \@params, [device => 'virtio-vga,edid=on,xres=1024,yres=768'], 'QEMU_VIDEO_DEVICE wins if both it and QEMUVGA are set' or diag explain \@params; ++ ++ delete $bmwqemu::vars{QEMUVGA}; ++ $bmwqemu::vars{QEMU_VIDEO_DEVICE_OPTIONS} = 'foo=bar'; ++ $backend->_set_graphics_backend(0); ++ is_deeply \@params, [device => 'virtio-vga,edid=on,xres=1024,yres=768,foo=bar'], 'QEMU_VIDEO_DEVICE_OPTIONS gets appended to EDID values' or diag explain \@params; + }; + + subtest 'execute arbitrary QMP command' => sub { +-- +2.37.1 + diff --git a/os-autoinst.spec b/os-autoinst.spec index cad3d50..0793519 100644 --- a/os-autoinst.spec +++ b/os-autoinst.spec @@ -30,9 +30,9 @@ %global github_owner os-autoinst %global github_name os-autoinst %global github_version 4.6 -%global github_commit eb3f4834fbce01a01cff993ca7eb94d929a85a83 +%global github_commit 436f134262416559c5b7248d4246cbfed67ae835 # if set, will be a post-release snapshot build, otherwise a 'normal' build -%global github_date 20220822 +%global github_date 20220923 %global shortcommit %(c=%{github_commit}; echo ${c:0:7}) Name: os-autoinst @@ -42,6 +42,9 @@ Summary: OS-level test automation License: GPLv2+ URL: https://os-autoinst.github.io/openQA/ Source0: https://github.com/%{github_owner}/%{github_name}/archive/%{github_commit}/%{github_name}-%{github_commit}.tar.gz +# Refactor video device output handling for qemu +# https://github.com/os-autoinst/os-autoinst/pull/2177 +Patch0: 0001-Consolidate-qemu-video-device-setting-deprecate-QEMU.patch # on SUSE this is conditional, for us it doesn't have to be but we # still use a macro just to keep build_requires similar for ease of @@ -98,6 +101,8 @@ Recommends: /usr/bin/qemu-img # Optional dependency for Python test API support Recommends: perl(Inline::Python) BuildRequires: %test_requires %test_version_only_requires +# For unbuffered output of Perl testsuite +BuildRequires: expect Requires: %main_requires Requires(pre): %{_bindir}/getent Requires(pre): %{_sbindir}/useradd @@ -180,7 +185,7 @@ export CI=1 export OPENQA_TEST_TIMEOUT_SCALE_CI=20 # Enable verbose test output as we can not store test artifacts within package # build environments in case of needing to investigate failures -export PROVE_ARGS="--timer -v" +export PROVE_ARGS="--timer -v --nocolor" # 00-compile-check-all.t fails if this is present and Perl::Critic is # not installed rm tools/lib/perlcritic/Perl/Critic/Policy/*.pm @@ -243,6 +248,10 @@ rm tools/lib/perlcritic/Perl/Critic/Policy/*.pm %files devel %changelog +* Fri Sep 23 2022 Adam Williamson - 4.6^20220923git436f134-1 +- Update to latest git +- Backport PR #2177 to clean up video device handling + * Tue Aug 23 2022 Adam Williamson - 4.6^20220822giteb3f483-1 - Update to latest git diff --git a/sources b/sources index 621ecf8..47aea29 100644 --- a/sources +++ b/sources @@ -1 +1 @@ -SHA512 (os-autoinst-d3d433bda958b473ba7c60bb434760566c0d356b.tar.gz) = 16aadb1ce6207e1e326412d820eb6b33a43a84ac0518462553b0b9bce308a3255f5f0f5b377b29318582848969a03db4d0ea3c8e1444698b09c9e44ae2d5a38e +SHA512 (os-autoinst-436f134262416559c5b7248d4246cbfed67ae835.tar.gz) = 7d2a6e57f74f865e29a1d69e825ee371d6911992ad6fd40df376ef88e6fcc064a2d24a5ba636696ec3ab2b8f1e137bbc34d8fa6d4cc3e4290c8c9dd914da6b1c