[virt-v2v wave 2 PATCH v1 00/16] replace QXL (and Cirrus) with standard VGA
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 RFC: https://listman.redhat.com/archives/libguestfs/2021-November/msg00007.html Please see the v1 changes (relative to the RFC posting) in the Notes sections of the individual patches. No actual guests have been converted for testing v1. Cc: Fabian Deutsch <fdeutsch@redhat.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Kashyap Chamarthy <kchamart@redhat.com> Thanks, Laszlo Laszlo Ersek (16): lib/types: introduce "Standard_VGA" constructor for "guestcaps_video_type" output: handle Standard_VGA in the libvirt XML and QEMU cmdline formats output: handle Standard_VGA in the OpenStack format output: handle Standard_VGA in the JSON format convert_linux: flip target display to Standard_VGA convert/windows_virtio: flip target display to Standard_VGA lib/create_ovf: fix "qxl_resourcetype" misnomer lib/create_ovf: place a standard VGA video device in the OVF xml lib/types: remove "QXL" constructor for "guestcaps_video_type" tests: remove the fake Windows guest drivers for the QXL device tests: remove the remaining QXL references (for completeness) lib/types: remove "Cirrus" constructor for "guestcaps_video_type" convert/windows_virtio: fix documentation of "install_drivers" return type lib/types: remove "guestcaps_video_type" lib/types: replace "source_video" type with plain "string" lib/types: remove the "source.s_video" field convert/convert_linux.ml | 3 +-- convert/convert_windows.ml | 2 -- convert/windows_virtio.ml | 18 ++------------- convert/windows_virtio.mli | 11 +++++----- input/input.ml | 2 -- input/parse_domain_from_vmx.ml | 8 ------- input/parse_libvirt_xml.ml | 16 -------------- lib/create_ovf.ml | 18 +++++++++------ lib/types.ml | 23 -------------------- lib/types.mli | 18 ++++----------- output/create_json.ml | 5 +---- output/create_libvirt_xml.ml | 4 +--- output/openstack_image_properties.ml | 5 +---- output/output.ml | 3 +-- test-data/fake-virtio-win/Makefile.am | 16 -------------- test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.cat | 1 - test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.inf | 2 -- test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.sys | 1 - test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxldd.dll | 1 - test-data/fake-virtio-win/drivers/amd64/Win7/qxl.cat | 1 - test-data/fake-virtio-win/drivers/amd64/Win7/qxl.inf | 2 -- test-data/fake-virtio-win/drivers/amd64/Win7/qxl.sys | 1 - test-data/fake-virtio-win/drivers/amd64/Win7/qxldd.dll | 1 - test-data/fake-virtio-win/drivers/i386/Win7/qxl.cat | 1 - test-data/fake-virtio-win/drivers/i386/Win7/qxl.inf | 2 -- test-data/fake-virtio-win/drivers/i386/Win7/qxl.sys | 1 - test-data/fake-virtio-win/drivers/i386/Win7/qxldd.dll | 1 - test-data/fake-virtio-win/drivers/i386/WinXP/qxl.cat | 1 - test-data/fake-virtio-win/drivers/i386/WinXP/qxl.inf | 2 -- test-data/fake-virtio-win/drivers/i386/WinXP/qxl.sys | 1 - test-data/fake-virtio-win/drivers/i386/WinXP/qxldd.dll | 1 - test-data/phony-guests/guests.xml.in | 2 +- tests/test-v2v-i-ova-formats.expected | 1 - tests/test-v2v-i-ova-gz.expected | 1 - tests/test-v2v-i-ova-snapshots.expected | 1 - tests/test-v2v-i-ova-snapshots.expected2 | 1 - tests/test-v2v-i-ova-subfolders.expected | 1 - tests/test-v2v-i-ova-subfolders.expected2 | 1 - tests/test-v2v-i-ova-tar.expected | 1 - tests/test-v2v-i-ova-tar.expected2 | 1 - tests/test-v2v-i-ova-two-disks.expected | 1 - tests/test-v2v-i-ova-two-disks.expected2 | 1 - tests/test-v2v-i-ova.xml | 2 +- tests/test-v2v-i-vmx-1.expected | 1 - tests/test-v2v-i-vmx-2.expected | 1 - tests/test-v2v-i-vmx-3.expected | 1 - tests/test-v2v-i-vmx-4.expected | 1 - tests/test-v2v-i-vmx-5.expected | 1 - tests/test-v2v-in-place.sh | 2 +- tests/test-v2v-o-rhv.ovf.expected | 2 +- tests/test-v2v-o-vdsm-options.ovf.expected | 2 +- tests/test-v2v-print-source.expected | 1 - tests/test-v2v-print-source.xml.in | 2 +- tests/test-v2v-windows-conversion.sh | 2 +- v2v/v2v_unit_tests.ml | 16 -------------- 55 files changed, 34 insertions(+), 184 deletions(-) delete mode 100644 test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.cat delete mode 100644 test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.inf delete mode 100644 test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.sys delete mode 100644 test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxldd.dll delete mode 100644 test-data/fake-virtio-win/drivers/amd64/Win7/qxl.cat delete mode 100644 test-data/fake-virtio-win/drivers/amd64/Win7/qxl.inf delete mode 100644 test-data/fake-virtio-win/drivers/amd64/Win7/qxl.sys delete mode 100644 test-data/fake-virtio-win/drivers/amd64/Win7/qxldd.dll delete mode 100644 test-data/fake-virtio-win/drivers/i386/Win7/qxl.cat delete mode 100644 test-data/fake-virtio-win/drivers/i386/Win7/qxl.inf delete mode 100644 test-data/fake-virtio-win/drivers/i386/Win7/qxl.sys delete mode 100644 test-data/fake-virtio-win/drivers/i386/Win7/qxldd.dll delete mode 100644 test-data/fake-virtio-win/drivers/i386/WinXP/qxl.cat delete mode 100644 test-data/fake-virtio-win/drivers/i386/WinXP/qxl.inf delete mode 100644 test-data/fake-virtio-win/drivers/i386/WinXP/qxl.sys delete mode 100644 test-data/fake-virtio-win/drivers/i386/WinXP/qxldd.dll base-commit: ab55f9432e774d83abe431cdf72b6b8e194fe9ec -- 2.19.1.3.g30247aa5d201
In order to replace QXL with Standard_VGA in small steps, bisectably, first just introduce the Standard_VGA constructor. Extend the string_of_video function at once ("stdvga"). The value returned by this function is only used for debug logging (indirectly, via the string_of_guestcaps function). The virt-v2v build flags turn "non-exhaustive pattern matching" warnings into errors. Match the new "Standard_VGA" constructor with a constant false assertion wherever an expression of type "guestcaps_video_type" is matched, in order to make this patch bisectable. These assertions will be gradually replaced with actual logic in the rest of this patch set. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v1: - drop the "convert/convert_linux.ml" and "convert/windows_virtio.ml" hunks, as those files don't match "guestcaps_video_type" expressions since commit b28cd1dcfeb4 ("Remove requested_guestcaps / rcaps", 2021-11-08) [Rich] - remove the "string_of_requested_guestcaps" reference from the commit message, due to the same commit [Rich] - contextual conflict, due to commit f0afc4395248 ("Remove guestcaps_block_type Virtio_SCSI", 2021-11-08), auto-resolved in "lib/types.mli" and "lib/types.ml" lib/types.ml | 3 ++- lib/types.mli | 2 +- output/create_json.ml | 1 + output/create_libvirt_xml.ml | 1 + output/openstack_image_properties.ml | 1 + output/output.ml | 6 +++++- 6 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/types.ml b/lib/types.ml index aeb7d71ae0a3..e7fc8b38787c 100644 --- a/lib/types.ml +++ b/lib/types.ml @@ -424,7 +424,7 @@ type guestcaps = { } and guestcaps_block_type = Virtio_blk | IDE and guestcaps_net_type = Virtio_net | E1000 | RTL8139 -and guestcaps_video_type = QXL | Cirrus +and guestcaps_video_type = Standard_VGA | QXL | Cirrus and guestcaps_machine = I440FX | Q35 | Virt let string_of_block_type = function @@ -435,6 +435,7 @@ let string_of_net_type = function | E1000 -> "e1000" | RTL8139 -> "rtl8139" let string_of_video = function + | Standard_VGA -> "stdvga" | QXL -> "qxl" | Cirrus -> "cirrus" let string_of_machine = function diff --git a/lib/types.mli b/lib/types.mli index 3aab1ea7e4f2..3a2ba9fecf9a 100644 --- a/lib/types.mli +++ b/lib/types.mli @@ -284,7 +284,7 @@ type guestcaps = { and guestcaps_block_type = Virtio_blk | IDE and guestcaps_net_type = Virtio_net | E1000 | RTL8139 -and guestcaps_video_type = QXL | Cirrus +and guestcaps_video_type = Standard_VGA | QXL | Cirrus and guestcaps_machine = I440FX | Q35 | Virt val string_of_guestcaps : guestcaps -> string diff --git a/output/create_json.ml b/output/create_json.ml index 211cd5d648c0..3cadf6b43378 100644 --- a/output/create_json.ml +++ b/output/create_json.ml @@ -203,6 +203,7 @@ let create_json_metadata source inspect | RTL8139 -> "rtl8139" in let video = match guestcaps.gcaps_video with + | Standard_VGA -> assert false | QXL -> "qxl" | Cirrus -> "cirrus" in let machine = diff --git a/output/create_libvirt_xml.ml b/output/create_libvirt_xml.ml index 64f1b0888cd6..ec1cd455ef13 100644 --- a/output/create_libvirt_xml.ml +++ b/output/create_libvirt_xml.ml @@ -421,6 +421,7 @@ let create_libvirt_xml ?pool source inspect let video = let video_model = match guestcaps.gcaps_video with + | Standard_VGA -> assert false | QXL -> e "model" [ "type", "qxl"; "ram", "65536" ] [] | Cirrus -> e "model" [ "type", "cirrus"; "vram", "9216" ] [] in append_attr ("heads", "1") video_model; diff --git a/output/openstack_image_properties.ml b/output/openstack_image_properties.ml index bca0766e28c1..72f3e9d9261d 100644 --- a/output/openstack_image_properties.ml +++ b/output/openstack_image_properties.ml @@ -43,6 +43,7 @@ let create source inspect { target_buses; guestcaps; target_firmware } = | RTL8139 -> "rtl8139"); "hw_video_model", (match guestcaps.gcaps_video with + | Standard_VGA -> assert false | QXL -> "qxl" | Cirrus -> "cirrus"); "hw_machine_type", diff --git a/output/output.ml b/output/output.ml index 9c2d8853bfef..93dbd4a62865 100644 --- a/output/output.ml +++ b/output/output.ml @@ -1539,7 +1539,11 @@ and qemu_finalize dir source inspect target_meta "addr=127.0.0.1"] ); arg "-vga" - (match guestcaps.gcaps_video with Cirrus -> "cirrus" | QXL -> "qxl") + (match guestcaps.gcaps_video with + | Standard_VGA -> assert false + | Cirrus -> "cirrus" + | QXL -> "qxl" + ) ); (* Add a sound card. *) -- 2.19.1.3.g30247aa5d201
Libvirt domain XML fragment (with the default video RAM size made explicit): <video> <model type='vga' vram='16384' heads='1'/> </video> Example QEMU cmdline option: -vga std References: - https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#VGA - https://libvirt.org/formatdomain.html#video-devices Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v1: - no change RFC: - This is where further patches should be added for the OVF, JSON and OpenStack output modes. output/create_libvirt_xml.ml | 2 +- output/output.ml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/output/create_libvirt_xml.ml b/output/create_libvirt_xml.ml index ec1cd455ef13..623755de7212 100644 --- a/output/create_libvirt_xml.ml +++ b/output/create_libvirt_xml.ml @@ -421,7 +421,7 @@ let create_libvirt_xml ?pool source inspect let video = let video_model = match guestcaps.gcaps_video with - | Standard_VGA -> assert false + | Standard_VGA -> e "model" [ "type", "vga"; "vram", "16384" ] [] | QXL -> e "model" [ "type", "qxl"; "ram", "65536" ] [] | Cirrus -> e "model" [ "type", "cirrus"; "vram", "9216" ] [] in append_attr ("heads", "1") video_model; diff --git a/output/output.ml b/output/output.ml index 93dbd4a62865..b27d53059712 100644 --- a/output/output.ml +++ b/output/output.ml @@ -1540,7 +1540,7 @@ and qemu_finalize dir source inspect target_meta ); arg "-vga" (match guestcaps.gcaps_video with - | Standard_VGA -> assert false + | Standard_VGA -> "std" | Cirrus -> "cirrus" | QXL -> "qxl" ) -- 2.19.1.3.g30247aa5d201
Convert the Standard_VGA value of "guestcaps.gcaps_video" to the string "vga", in the OpenStack image properties. Reference: https://docs.openstack.org/glance/xena/admin/useful-image-properties.html Cc: Kashyap Chamarthy <kchamart@redhat.com> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v1: - new in v1 output/openstack_image_properties.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/output/openstack_image_properties.ml b/output/openstack_image_properties.ml index 72f3e9d9261d..9d466ecfbf0f 100644 --- a/output/openstack_image_properties.ml +++ b/output/openstack_image_properties.ml @@ -43,7 +43,7 @@ let create source inspect { target_buses; guestcaps; target_firmware } = | RTL8139 -> "rtl8139"); "hw_video_model", (match guestcaps.gcaps_video with - | Standard_VGA -> assert false + | Standard_VGA -> "vga" | QXL -> "qxl" | Cirrus -> "cirrus"); "hw_machine_type", -- 2.19.1.3.g30247aa5d201
On Sat, Nov 13, 2021 at 11:29:46PM +0100, Laszlo Ersek wrote:
Convert the Standard_VGA value of "guestcaps.gcaps_video" to the string "vga", in the OpenStack image properties.
Hi, Your patch reminds me that I neglected to update upstream Glance documentation to add "bochs" video model when I added support for it [1] in Nova. So I just submitted[2] a docs patch to Glance to rectify that. Thanks for the reminder. :-) [1] https://opendev.org/openstack/nova/commit/c590843974 (Allow 'bochs' as a display device option) [2] https://review.opendev.org/c/openstack/glance/+/817950 (Add "bochs" as a possible video model)
Reference: https://docs.openstack.org/glance/xena/admin/useful-image-properties.html
Cc: Kashyap Chamarthy <kchamart@redhat.com> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> ---
Notes: v1:
- new in v1
output/openstack_image_properties.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/output/openstack_image_properties.ml b/output/openstack_image_properties.ml index 72f3e9d9261d..9d466ecfbf0f 100644 --- a/output/openstack_image_properties.ml +++ b/output/openstack_image_properties.ml @@ -43,7 +43,7 @@ let create source inspect { target_buses; guestcaps; target_firmware } = | RTL8139 -> "rtl8139"); "hw_video_model", (match guestcaps.gcaps_video with - | Standard_VGA -> assert false + | Standard_VGA -> "vga" | QXL -> "qxl" | Cirrus -> "cirrus");
Please also add an entry for "bochs" video model. With that amended, FWIW: Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>
"hw_machine_type", -- 2.19.1.3.g30247aa5d201
-- /kashyap
Hi Kashyap, On 11/15/21 14:54, Kashyap Chamarthy wrote:
On Sat, Nov 13, 2021 at 11:29:46PM +0100, Laszlo Ersek wrote:
Convert the Standard_VGA value of "guestcaps.gcaps_video" to the string "vga", in the OpenStack image properties.
Hi,
Your patch reminds me that I neglected to update upstream Glance documentation to add "bochs" video model when I added support for it [1] in Nova. So I just submitted[2] a docs patch to Glance to rectify that.
Thanks for the reminder. :-)
[1] https://opendev.org/openstack/nova/commit/c590843974 (Allow 'bochs' as a display device option) [2] https://review.opendev.org/c/openstack/glance/+/817950 (Add "bochs" as a possible video model)
Reference: https://docs.openstack.org/glance/xena/admin/useful-image-properties.html
Cc: Kashyap Chamarthy <kchamart@redhat.com> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> ---
Notes: v1:
- new in v1
output/openstack_image_properties.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/output/openstack_image_properties.ml b/output/openstack_image_properties.ml index 72f3e9d9261d..9d466ecfbf0f 100644 --- a/output/openstack_image_properties.ml +++ b/output/openstack_image_properties.ml @@ -43,7 +43,7 @@ let create source inspect { target_buses; guestcaps; target_firmware } = | RTL8139 -> "rtl8139"); "hw_video_model", (match guestcaps.gcaps_video with - | Standard_VGA -> assert false + | Standard_VGA -> "vga" | QXL -> "qxl" | Cirrus -> "cirrus");
Please also add an entry for "bochs" video model.
That is not useful for us in this BZ / patch series. The purpose of virt-v2v here is that the converted domain start with *some* usable video device. After discussing the potential options with Gerd at length (especially based on his blog post), it became clear that VGA was the best choice -- smaller attack surface than QXL, performance-wise just as fine for the above-stated virt-v2v purpose, and universally recognized by all guests we care about. Therefore VGA replaces QXL and also the older (compat) Cirrus. The whole idea is to simplify / unify as much as possible. Adding a mapping for Bochs is not useful here, as virt-v2v will not intend (in the foreseeable future) to produce such output (= converted) domains. Once the converted domain lives in OpenStack, the admin can easily change the video device, but that's beyond virt-v2v's mandate.
With that amended, FWIW:
Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>
So, can I take your R-b only for the correctness of "vga"? ("Completeness", with regard to Bochs for example, is not a goal here, again.) Thank you! Laszlo
"hw_machine_type", -- 2.19.1.3.g30247aa5d201
On Tue, Nov 16, 2021 at 11:31:48AM +0100, Laszlo Ersek wrote:
Hi Kashyap,
Heya,
On 11/15/21 14:54, Kashyap Chamarthy wrote:
[...]
diff --git a/output/openstack_image_properties.ml b/output/openstack_image_properties.ml index 72f3e9d9261d..9d466ecfbf0f 100644 --- a/output/openstack_image_properties.ml +++ b/output/openstack_image_properties.ml @@ -43,7 +43,7 @@ let create source inspect { target_buses; guestcaps; target_firmware } = | RTL8139 -> "rtl8139"); "hw_video_model", (match guestcaps.gcaps_video with - | Standard_VGA -> assert false + | Standard_VGA -> "vga" | QXL -> "qxl" | Cirrus -> "cirrus");
Please also add an entry for "bochs" video model.
That is not useful for us in this BZ / patch series. The purpose of virt-v2v here is that the converted domain start with *some* usable video device.
Ah, noted.
After discussing the potential options with Gerd at length (especially based on his blog post), it became clear that VGA was the best choice -- smaller attack surface than QXL, performance-wise just as fine for the above-stated virt-v2v purpose, and universally recognized by all guests we care about. Therefore VGA replaces QXL and also the older (compat) Cirrus. The whole idea is to simplify / unify as much as possible.
Understood; and it makes full sense. FWIW, a few months ago I too had a disucssion with Gerd, based on his famous blog post, to pick a saner default for OpenStack (it uses the undersirable "cirrus" now). I've recently filed an RFE[1] to switch to "virtio" as the default, because: - Any Linux guest with kernel 4.4 (or a kernel with the Linux 'virtio-vgpu' driver backported) can take full advantage of the 'virtio' display device. - In case your guest kernel does _not_ have the kernel driver (called "virtio-gpu.ko"), no problem: the 'virtio' display device will gracefully fallback to VGA compatibility mode. [1] https://blueprints.launchpad.net/nova/+spec/virtio-as-default-display-device
Adding a mapping for Bochs is not useful here, as virt-v2v will not intend (in the foreseeable future) to produce such output (= converted) domains. Once the converted domain lives in OpenStack, the admin can easily change the video device, but that's beyond virt-v2v's mandate.
With that amended, FWIW:
Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>
So, can I take your R-b only for the correctness of "vga"?
Yes, please - for whatever it's little worth :-)
("Completeness", with regard to Bochs for example, is not a goal here, again.)
Yep, understood. [...] -- /kashyap
The JSON output mode was added in commit fba6a498d472 ("v2v: add -o json output mode", 2019-04-01). The format that it uses is by convention (aka "ad-hoc"); not governed by any public specification. Map the Standard_VGA value of "guestcaps.gcaps_video" to the "vga" string, as this is (a) straightforward and (b) consistent with the OpenStack and libvirt device model names. Cc: Fabian Deutsch <fdeutsch@redhat.com> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v1: - new in v1 output/create_json.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/output/create_json.ml b/output/create_json.ml index 3cadf6b43378..ac044630a0c6 100644 --- a/output/create_json.ml +++ b/output/create_json.ml @@ -203,7 +203,7 @@ let create_json_metadata source inspect | RTL8139 -> "rtl8139" in let video = match guestcaps.gcaps_video with - | Standard_VGA -> assert false + | Standard_VGA -> "vga" | QXL -> "qxl" | Cirrus -> "cirrus" in let machine = -- 2.19.1.3.g30247aa5d201
Policy change: pick Standard_VGA over QXL for the video type in the output (Linux) domain, always. For Standard_VGA, use the "modesetting" X.org driver. (In a Fedora 34 guest using standard VGA video, and having no explicit X.org config file, the X.org server logs the following (excerpt):
Markers: (--) probed, (**) from config file, (==) default setting, (++) from command line, (!!) notice, (II) informational, (WW) warning, (EE) error, (NI) not implemented, (??) unknown. (==) Matched modesetting as autoconfigured driver 0 (==) Matched fbdev as autoconfigured driver 1 (==) Matched vesa as autoconfigured driver 2 (II) LoadModule: "modesetting" (II) Loading /usr/lib64/xorg/modules/drivers/modesetting_drv.so (II) LoadModule: "fbdev" (II) Loading /usr/lib64/xorg/modules/drivers/fbdev_drv.so (II) LoadModule: "vesa" (II) Loading /usr/lib64/xorg/modules/drivers/vesa_drv.so (II) Loading sub module "fbdevhw" (II) LoadModule: "fbdevhw" (II) Loading /usr/lib64/xorg/modules/libfbdevhw.so (II) UnloadModule: "fbdev" (II) Unloading fbdev (II) UnloadSubModule: "fbdevhw" (II) Unloading fbdevhw (II) UnloadModule: "vesa" (II) Unloading vesa
This tells us that the standard VGA device model is driven by the "modesetting" driver.) Cc: Gerd Hoffmann <kraxel@redhat.com> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Acked-by: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v1: - reimplement the following RFC patches, taken together: - convert_linux: use "modesetting" X.org driver for Standard_VGA - convert_linux: flip default target display to Standard_VGA due to commit b28cd1dcfeb4 ("Remove requested_guestcaps / rcaps", 2021-11-08) [Rich] - pick up Gerd's ACK for choosing the "modesetting" X.org driver. convert/convert_linux.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml index 41bc4218e948..f0213f06eec2 100644 --- a/convert/convert_linux.ml +++ b/convert/convert_linux.ml @@ -148,7 +148,7 @@ let convert (g : G.guestfs) source inspect keep_serial_console _ = let guestcaps = { gcaps_block_bus = block_type; gcaps_net_bus = net_type; - gcaps_video = QXL; + gcaps_video = Standard_VGA; gcaps_virtio_rng = kernel.ki_supports_virtio_rng; gcaps_virtio_balloon = kernel.ki_supports_virtio_balloon; gcaps_isa_pvpanic = kernel.ki_supports_isa_pvpanic; @@ -819,7 +819,7 @@ let convert (g : G.guestfs) source inspect keep_serial_console _ = true and configure_display_driver () = - let video_driver = "qxl" in + let video_driver = "modesetting" in let updated = ref false in -- 2.19.1.3.g30247aa5d201
Policy change: pick Standard_VGA over either QXL or Cirrus for the video type in the output (Windows) domain, always. If the subject Windows version is entirely unsupported by "virtio-win.iso", continue warning the user, as IDE and RTL8139 are still considered inferior to virtio-blk and virtio-net, respectively. No warning is needed when only the QXL driver is missing for the subject Windows version, as Standard_VGA is not worse than QXL. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v1: - Reimplement the RFC patch "convert/windows_virtio: flip default target display to Standard_VGA" on top of commit b28cd1dcfeb4 ("Remove requested_guestcaps / rcaps", 2021-11-08) [Rich] - The test cases listed previously (below, under "RFC") no longer break. RFC: - This patch breaks the following test cases: - test-v2v-o-glance.sh - test-v2v-o-json.sh - test-v2v-o-openstack.sh That's because we still have the initial "assert false" expressions for Standard_VGA in the OpenStack and JSON (... and OVF) producers, from patch 'lib/types: introduce "Standard_VGA" constructor for "guestcaps_video_type'. convert/windows_virtio.ml | 18 ++---------------- tests/test-v2v-i-ova.xml | 2 +- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/convert/windows_virtio.ml b/convert/windows_virtio.ml index 08db4ad6995c..77aeac2d4a9f 100644 --- a/convert/windows_virtio.ml +++ b/convert/windows_virtio.ml @@ -53,7 +53,7 @@ let rec install_drivers ((g, _) as reg) inspect = warning (f_"there are no virtio drivers available for this version of Windows (%d.%d %s %s). virt-v2v looks for drivers in %s\n\nThe guest will be configured to use slower emulated devices.") inspect.i_major_version inspect.i_minor_version inspect.i_arch inspect.i_product_variant virtio_win; - (IDE, RTL8139, Cirrus, false, false, false, false) + (IDE, RTL8139, Standard_VGA, false, false, false, false) ) else ( (* Can we install the block driver? *) @@ -103,27 +103,13 @@ let rec install_drivers ((g, _) as reg) inspect = else Virtio_net in - (* Can we install the QXL driver? *) - let video : guestcaps_video_type = - let has_qxl = - g#exists (driverdir // "qxl.inf") || - g#exists (driverdir // "qxldod.inf") in - if not has_qxl then ( - warning (f_"there is no QXL driver for this version of Windows (%d.%d %s). virt-v2v looks for this driver in %s\n\nThe guest will be configured to use a basic VGA display driver.") - inspect.i_major_version inspect.i_minor_version - inspect.i_arch virtio_win; - Cirrus - ) - else - QXL in - (* Did we install the miscellaneous drivers? *) let virtio_rng_supported = g#exists (driverdir // "viorng.inf") in let virtio_ballon_supported = g#exists (driverdir // "balloon.inf") in let isa_pvpanic_supported = g#exists (driverdir // "pvpanic.inf") in let virtio_socket_supported = g#exists (driverdir // "viosock.inf") in - (block, net, video, + (block, net, Standard_VGA, virtio_rng_supported, virtio_ballon_supported, isa_pvpanic_supported, virtio_socket_supported) ) diff --git a/tests/test-v2v-i-ova.xml b/tests/test-v2v-i-ova.xml index 30f52f557d9f..d7383905fdc0 100644 --- a/tests/test-v2v-i-ova.xml +++ b/tests/test-v2v-i-ova.xml @@ -39,7 +39,7 @@ <model type='virtio'/> </interface> <video> - <model type='qxl' ram='65536' heads='1'/> + <model type='vga' vram='16384' heads='1'/> </video> <graphics type='vnc' autoport='yes' port='-1'/> <rng model='virtio'> -- 2.19.1.3.g30247aa5d201
The OVF resource types 32768 and 20 have nothing to do with QXL (or with any other video controller); they stand for the *monitor* resource type. Rename the "qxl_resourcetype" variable to "monitor_resourcetype". Also document the ovirt-engine bugzilla for which resource type 32768 had been introduced to ovirt-engine (RHBZ#1534644). References: - ovirt-engine commit 4dc21c0fcf3b ("core: align hardware resource type with ovf specification", 2018-02-14) - [Libguestfs] specifying a standard VGA video controller in OVF for oVirt https://listman.redhat.com/archives/libguestfs/2021-November/msg00149.html Fixes: 5d078f1f324bc121f3aa29cb87768ffa69385b6f Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v1: - new in v1 lib/create_ovf.ml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/create_ovf.ml b/lib/create_ovf.ml index 3991b66dd95d..8c8ef43a42cd 100644 --- a/lib/create_ovf.ml +++ b/lib/create_ovf.ml @@ -679,14 +679,15 @@ let rec create_ovf source inspect * See RHBZ#1213701 and RHBZ#1211231 for the reasoning * behind that. *) - let qxl_resourcetype = + let monitor_resourcetype = match ovf_flavour with - | OVirt -> 32768 (* RHBZ#1598715 *) + | OVirt -> 32768 (* RHBZ#1598715, RHBZ#1534644 *) | RHVExportStorageDomain -> 20 in e "Item" [] [ e "rasd:Caption" [] [PCData "Graphical Controller"]; e "rasd:InstanceId" [] [PCData (uuidgen ())]; - e "rasd:ResourceType" [] [PCData (string_of_int qxl_resourcetype)]; + e "rasd:ResourceType" [] + [PCData (string_of_int monitor_resourcetype)]; e "Type" [] [PCData "video"]; e "rasd:VirtualQuantity" [] [PCData "1"]; e "rasd:Device" [] [PCData "qxl"]; -- 2.19.1.3.g30247aa5d201
As of ovirt-engine commit daca2ca6cd91, ovirt-engine ignores <rasd:Device>qxl</rasd:Device> entirely; ovirt-engine picks the QXL device based on other factors. In preparation for ovirt-engine honoring <rasd:Device>, and in particular for selecting VGA over QXL as a policy change, replace the element's contents "qxl" with "vga". (The latter is a part of ovirt-engine's VmDeviceType enum type just the same.) Reference: [Libguestfs] specifying a standard VGA video controller in OVF for oVirt https://listman.redhat.com/archives/libguestfs/2021-November/msg00149.html Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v1: - new in v1 lib/create_ovf.ml | 11 +++++++---- tests/test-v2v-o-rhv.ovf.expected | 2 +- tests/test-v2v-o-vdsm-options.ovf.expected | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/create_ovf.ml b/lib/create_ovf.ml index 8c8ef43a42cd..6ccea1e9e6d7 100644 --- a/lib/create_ovf.ml +++ b/lib/create_ovf.ml @@ -675,9 +675,12 @@ let rec create_ovf source inspect e "rasd:UsbPolicy" [] [PCData "Disabled"]; ]; - (* We always add a qxl device when outputting to RHV. - * See RHBZ#1213701 and RHBZ#1211231 for the reasoning - * behind that. + (* We always add a standard VGA-compatible video controller when + * outputting to RHV. See RHBZ#1213701 and RHBZ#1211231 for the + * reasoning behind that. The device model used to be QXL previously, + * but only the unaccelerated standard VGA framebuffer is needed; so + * the (simpler) standard VGA device itself suffices. Refer to + * RHBZ#1961107. *) let monitor_resourcetype = match ovf_flavour with @@ -690,7 +693,7 @@ let rec create_ovf source inspect [PCData (string_of_int monitor_resourcetype)]; e "Type" [] [PCData "video"]; e "rasd:VirtualQuantity" [] [PCData "1"]; - e "rasd:Device" [] [PCData "qxl"]; + e "rasd:Device" [] [PCData "vga"]; ] ]; diff --git a/tests/test-v2v-o-rhv.ovf.expected b/tests/test-v2v-o-rhv.ovf.expected index f8aa07c45d4c..a8f5ebdfc7e9 100644 --- a/tests/test-v2v-o-rhv.ovf.expected +++ b/tests/test-v2v-o-rhv.ovf.expected @@ -60,7 +60,7 @@ <rasd:ResourceType>20</rasd:ResourceType> <Type>video</Type> <rasd:VirtualQuantity>1</rasd:VirtualQuantity> - <rasd:Device>qxl</rasd:Device> + <rasd:Device>vga</rasd:Device> </Item> <Item> <rasd:Caption>RNG Device</rasd:Caption> diff --git a/tests/test-v2v-o-vdsm-options.ovf.expected b/tests/test-v2v-o-vdsm-options.ovf.expected index f861071c08c3..a2a26a839462 100644 --- a/tests/test-v2v-o-vdsm-options.ovf.expected +++ b/tests/test-v2v-o-vdsm-options.ovf.expected @@ -60,7 +60,7 @@ <rasd:ResourceType>32768</rasd:ResourceType> <Type>video</Type> <rasd:VirtualQuantity>1</rasd:VirtualQuantity> - <rasd:Device>qxl</rasd:Device> + <rasd:Device>vga</rasd:Device> </Item> <Item> <rasd:Caption>RNG Device</rasd:Caption> -- 2.19.1.3.g30247aa5d201
On 11/13/21 23:29, Laszlo Ersek wrote:
> As of ovirt-engine commit daca2ca6cd91, ovirt-engine ignores
>
> <rasd:Device>qxl</rasd:Device>
>
> entirely; ovirt-engine picks the QXL device based on other factors. In
> preparation for ovirt-engine honoring <rasd:Device>, and in particular for
> selecting VGA over QXL as a policy change, replace the element's contents
> "qxl" with "vga". (The latter is a part of ovirt-engine's VmDeviceType
> enum type just the same.)
>
> Reference:
> [Libguestfs] specifying a standard VGA video controller in OVF for oVirt
> https://listman.redhat.com/archives/libguestfs/2021-November/msg00149.html
>
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> v1:
>
> - new in v1
>
> lib/create_ovf.ml | 11 +++++++----
> tests/test-v2v-o-rhv.ovf.expected | 2 +-
> tests/test-v2v-o-vdsm-options.ovf.expected | 2 +-
> 3 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/lib/create_ovf.ml b/lib/create_ovf.ml
> index 8c8ef43a42cd..6ccea1e9e6d7 100644
> --- a/lib/create_ovf.ml
> +++ b/lib/create_ovf.ml
> @@ -675,9 +675,12 @@ let rec create_ovf source inspect
> e "rasd:UsbPolicy" [] [PCData "Disabled"];
> ];
>
> - (* We always add a qxl device when outputting to RHV.
> - * See RHBZ#1213701 and RHBZ#1211231 for the reasoning
> - * behind that.
> + (* We always add a standard VGA-compatible video controller when
> + * outputting to RHV. See RHBZ#1213701 and RHBZ#1211231 for the
> + * reasoning behind that. The device model used to be QXL previously,
> + * but only the unaccelerated standard VGA framebuffer is needed; so
> + * the (simpler) standard VGA device itself suffices. Refer to
> + * RHBZ#1961107.
> *)
> let monitor_resourcetype =
> match ovf_flavour with
> @@ -690,7 +693,7 @@ let rec create_ovf source inspect
> [PCData (string_of_int monitor_resourcetype)];
> e "Type" [] [PCData "video"];
> e "rasd:VirtualQuantity" [] [PCData "1"];
> - e "rasd:Device" [] [PCData "qxl"];
> + e "rasd:Device" [] [PCData "vga"];
> ]
> ];
>
> diff --git a/tests/test-v2v-o-rhv.ovf.expected b/tests/test-v2v-o-rhv.ovf.expected
> index f8aa07c45d4c..a8f5ebdfc7e9 100644
> --- a/tests/test-v2v-o-rhv.ovf.expected
> +++ b/tests/test-v2v-o-rhv.ovf.expected
> @@ -60,7 +60,7 @@
> <rasd:ResourceType>20</rasd:ResourceType>
> <Type>video</Type>
> <rasd:VirtualQuantity>1</rasd:VirtualQuantity>
> - <rasd:Device>qxl</rasd:Device>
> + <rasd:Device>vga</rasd:Device>
> </Item>
> <Item>
> <rasd:Caption>RNG Device</rasd:Caption>
> diff --git a/tests/test-v2v-o-vdsm-options.ovf.expected b/tests/test-v2v-o-vdsm-options.ovf.expected
> index f861071c08c3..a2a26a839462 100644
> --- a/tests/test-v2v-o-vdsm-options.ovf.expected
> +++ b/tests/test-v2v-o-vdsm-options.ovf.expected
> @@ -60,7 +60,7 @@
> <rasd:ResourceType>32768</rasd:ResourceType>
> <Type>video</Type>
> <rasd:VirtualQuantity>1</rasd:VirtualQuantity>
> - <rasd:Device>qxl</rasd:Device>
> + <rasd:Device>vga</rasd:Device>
> </Item>
> <Item>
> <rasd:Caption>RNG Device</rasd:Caption>
>
Based on feedback from Liran and Arik, here:
https://bugzilla.redhat.com/show_bug.cgi?id=1976607#c3
https://bugzilla.redhat.com/show_bug.cgi?id=1976607#c4
I should use
<Device>vga</Device>
not
<rasd:Device>vga</rasd:Device>
and then ovirt-engine already has code to handle that.
Liran: in comment 3 on the above BZ, you provide the snippet
<Item>
<rasd:Caption>Graphical Controller</rasd:Caption>
<rasd:InstanceID>2834786b-e9d0-4419-8e6a-c4843fed2f9c</rasd:InstanceID>
<rasd:ResourceType>32768</rasd:ResourceType>
<rasd:VirtualQuantity>1</rasd:VirtualQuantity>
<rasd:SinglePciQxl>false</rasd:SinglePciQxl>
<Type>video</Type>
<Device>vga</Device>
<rasd:Address></rasd:Address>
<BootOrder>0</BootOrder>
<IsPlugged>true</IsPlugged>
<IsReadOnly>false</IsReadOnly>
<Alias></Alias>
<SpecParams>
<vram>16384</vram>
</SpecParams>
</Item>
>From these, virt-v2v does not provide the following at all:
- rasd:SinglePciQxl
- rasd:Address
- BootOrder
- IsPlugged
- IsReadOnly
- Alias
- SpecParams (with child node "vram")
Are any of these required, in addition to <Device>vga</Device>?
Thanks,
Laszlo
On Tue, Nov 23, 2021 at 2:36 PM Laszlo Ersek <lersek@redhat.com> wrote:
On 11/13/21 23:29, Laszlo Ersek wrote:
As of ovirt-engine commit daca2ca6cd91, ovirt-engine ignores
<rasd:Device>qxl</rasd:Device>
entirely; ovirt-engine picks the QXL device based on other factors. In preparation for ovirt-engine honoring <rasd:Device>, and in particular for selecting VGA over QXL as a policy change, replace the element's contents "qxl" with "vga". (The latter is a part of ovirt-engine's VmDeviceType enum type just the same.)
Reference: [Libguestfs] specifying a standard VGA video controller in OVF for oVirt
https://listman.redhat.com/archives/libguestfs/2021-November/msg00149.html
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> ---
Notes: v1:
- new in v1
lib/create_ovf.ml | 11 +++++++---- tests/test-v2v-o-rhv.ovf.expected | 2 +- tests/test-v2v-o-vdsm-options.ovf.expected | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/lib/create_ovf.ml b/lib/create_ovf.ml index 8c8ef43a42cd..6ccea1e9e6d7 100644 --- a/lib/create_ovf.ml +++ b/lib/create_ovf.ml @@ -675,9 +675,12 @@ let rec create_ovf source inspect e "rasd:UsbPolicy" [] [PCData "Disabled"]; ];
- (* We always add a qxl device when outputting to RHV. - * See RHBZ#1213701 and RHBZ#1211231 for the reasoning - * behind that. + (* We always add a standard VGA-compatible video controller when + * outputting to RHV. See RHBZ#1213701 and RHBZ#1211231 for the + * reasoning behind that. The device model used to be QXL
previously,
+ * but only the unaccelerated standard VGA framebuffer is needed; so + * the (simpler) standard VGA device itself suffices. Refer to + * RHBZ#1961107. *) let monitor_resourcetype = match ovf_flavour with @@ -690,7 +693,7 @@ let rec create_ovf source inspect [PCData (string_of_int monitor_resourcetype)]; e "Type" [] [PCData "video"]; e "rasd:VirtualQuantity" [] [PCData "1"]; - e "rasd:Device" [] [PCData "qxl"]; + e "rasd:Device" [] [PCData "vga"]; ] ];
diff --git a/tests/test-v2v-o-rhv.ovf.expected b/tests/test-v2v-o-rhv.ovf.expected index f8aa07c45d4c..a8f5ebdfc7e9 100644 --- a/tests/test-v2v-o-rhv.ovf.expected +++ b/tests/test-v2v-o-rhv.ovf.expected @@ -60,7 +60,7 @@ <rasd:ResourceType>20</rasd:ResourceType> <Type>video</Type> <rasd:VirtualQuantity>1</rasd:VirtualQuantity> - <rasd:Device>qxl</rasd:Device> + <rasd:Device>vga</rasd:Device> </Item> <Item> <rasd:Caption>RNG Device</rasd:Caption> diff --git a/tests/test-v2v-o-vdsm-options.ovf.expected b/tests/test-v2v-o-vdsm-options.ovf.expected index f861071c08c3..a2a26a839462 100644 --- a/tests/test-v2v-o-vdsm-options.ovf.expected +++ b/tests/test-v2v-o-vdsm-options.ovf.expected @@ -60,7 +60,7 @@ <rasd:ResourceType>32768</rasd:ResourceType> <Type>video</Type> <rasd:VirtualQuantity>1</rasd:VirtualQuantity> - <rasd:Device>qxl</rasd:Device> + <rasd:Device>vga</rasd:Device> </Item> <Item> <rasd:Caption>RNG Device</rasd:Caption>
Based on feedback from Liran and Arik, here:
https://bugzilla.redhat.com/show_bug.cgi?id=1976607#c3 https://bugzilla.redhat.com/show_bug.cgi?id=1976607#c4
I should use
<Device>vga</Device>
not
<rasd:Device>vga</rasd:Device>
and then ovirt-engine already has code to handle that.
Liran: in comment 3 on the above BZ, you provide the snippet
<Item> <rasd:Caption>Graphical Controller</rasd:Caption> <rasd:InstanceID>2834786b-e9d0-4419-8e6a-c4843fed2f9c</rasd:InstanceID> <rasd:ResourceType>32768</rasd:ResourceType> <rasd:VirtualQuantity>1</rasd:VirtualQuantity> <rasd:SinglePciQxl>false</rasd:SinglePciQxl> <Type>video</Type> <Device>vga</Device> <rasd:Address></rasd:Address> <BootOrder>0</BootOrder> <IsPlugged>true</IsPlugged> <IsReadOnly>false</IsReadOnly> <Alias></Alias> <SpecParams> <vram>16384</vram> </SpecParams> </Item>
From these, virt-v2v does not provide the following at all: - rasd:SinglePciQxl - rasd:Address - BootOrder - IsPlugged - IsReadOnly - Alias - SpecParams (with child node "vram")
Are any of these required, in addition to <Device>vga</Device>?
It will auto set some of the values on import. For being extra careful I checked a VM that I exported omitting these values from the VGA item and it got imported back well(+ran). This was the snippet for the device: <Item> <rasd:Caption>Graphical Controller</rasd:Caption> <rasd:InstanceID>2834786b-e9d0-4419-8e6a-c4843fed2f9c</rasd:InstanceID> <rasd:ResourceType>32768</rasd:ResourceType> <rasd:VirtualQuantity>1</rasd:VirtualQuantity> <Type>video</Type> <Device>vga</Device> </Item>
Thanks, Laszlo
On 11/23/21 17:16, Liran Rotenberg wrote:
On Tue, Nov 23, 2021 at 2:36 PM Laszlo Ersek <lersek@redhat.com> wrote:
On 11/13/21 23:29, Laszlo Ersek wrote:
As of ovirt-engine commit daca2ca6cd91, ovirt-engine ignores
<rasd:Device>qxl</rasd:Device>
entirely; ovirt-engine picks the QXL device based on other factors. In preparation for ovirt-engine honoring <rasd:Device>, and in particular for selecting VGA over QXL as a policy change, replace the element's contents "qxl" with "vga". (The latter is a part of ovirt-engine's VmDeviceType enum type just the same.)
Reference: [Libguestfs] specifying a standard VGA video controller in OVF for oVirt
https://listman.redhat.com/archives/libguestfs/2021-November/msg00149.html
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> ---
Notes: v1:
- new in v1
lib/create_ovf.ml | 11 +++++++---- tests/test-v2v-o-rhv.ovf.expected | 2 +- tests/test-v2v-o-vdsm-options.ovf.expected | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/lib/create_ovf.ml b/lib/create_ovf.ml index 8c8ef43a42cd..6ccea1e9e6d7 100644 --- a/lib/create_ovf.ml +++ b/lib/create_ovf.ml @@ -675,9 +675,12 @@ let rec create_ovf source inspect e "rasd:UsbPolicy" [] [PCData "Disabled"]; ];
- (* We always add a qxl device when outputting to RHV. - * See RHBZ#1213701 and RHBZ#1211231 for the reasoning - * behind that. + (* We always add a standard VGA-compatible video controller when + * outputting to RHV. See RHBZ#1213701 and RHBZ#1211231 for the + * reasoning behind that. The device model used to be QXL
previously,
+ * but only the unaccelerated standard VGA framebuffer is needed; so + * the (simpler) standard VGA device itself suffices. Refer to + * RHBZ#1961107. *) let monitor_resourcetype = match ovf_flavour with @@ -690,7 +693,7 @@ let rec create_ovf source inspect [PCData (string_of_int monitor_resourcetype)]; e "Type" [] [PCData "video"]; e "rasd:VirtualQuantity" [] [PCData "1"]; - e "rasd:Device" [] [PCData "qxl"]; + e "rasd:Device" [] [PCData "vga"]; ] ];
diff --git a/tests/test-v2v-o-rhv.ovf.expected b/tests/test-v2v-o-rhv.ovf.expected index f8aa07c45d4c..a8f5ebdfc7e9 100644 --- a/tests/test-v2v-o-rhv.ovf.expected +++ b/tests/test-v2v-o-rhv.ovf.expected @@ -60,7 +60,7 @@ <rasd:ResourceType>20</rasd:ResourceType> <Type>video</Type> <rasd:VirtualQuantity>1</rasd:VirtualQuantity> - <rasd:Device>qxl</rasd:Device> + <rasd:Device>vga</rasd:Device> </Item> <Item> <rasd:Caption>RNG Device</rasd:Caption> diff --git a/tests/test-v2v-o-vdsm-options.ovf.expected b/tests/test-v2v-o-vdsm-options.ovf.expected index f861071c08c3..a2a26a839462 100644 --- a/tests/test-v2v-o-vdsm-options.ovf.expected +++ b/tests/test-v2v-o-vdsm-options.ovf.expected @@ -60,7 +60,7 @@ <rasd:ResourceType>32768</rasd:ResourceType> <Type>video</Type> <rasd:VirtualQuantity>1</rasd:VirtualQuantity> - <rasd:Device>qxl</rasd:Device> + <rasd:Device>vga</rasd:Device> </Item> <Item> <rasd:Caption>RNG Device</rasd:Caption>
Based on feedback from Liran and Arik, here:
https://bugzilla.redhat.com/show_bug.cgi?id=1976607#c3 https://bugzilla.redhat.com/show_bug.cgi?id=1976607#c4
I should use
<Device>vga</Device>
not
<rasd:Device>vga</rasd:Device>
and then ovirt-engine already has code to handle that.
Liran: in comment 3 on the above BZ, you provide the snippet
<Item> <rasd:Caption>Graphical Controller</rasd:Caption> <rasd:InstanceID>2834786b-e9d0-4419-8e6a-c4843fed2f9c</rasd:InstanceID> <rasd:ResourceType>32768</rasd:ResourceType> <rasd:VirtualQuantity>1</rasd:VirtualQuantity> <rasd:SinglePciQxl>false</rasd:SinglePciQxl> <Type>video</Type> <Device>vga</Device> <rasd:Address></rasd:Address> <BootOrder>0</BootOrder> <IsPlugged>true</IsPlugged> <IsReadOnly>false</IsReadOnly> <Alias></Alias> <SpecParams> <vram>16384</vram> </SpecParams> </Item>
From these, virt-v2v does not provide the following at all: - rasd:SinglePciQxl - rasd:Address - BootOrder - IsPlugged - IsReadOnly - Alias - SpecParams (with child node "vram")
Are any of these required, in addition to <Device>vga</Device>?
It will auto set some of the values on import. For being extra careful I checked a VM that I exported omitting these values from the VGA item and it got imported back well(+ran). This was the snippet for the device: <Item> <rasd:Caption>Graphical Controller</rasd:Caption> <rasd:InstanceID>2834786b-e9d0-4419-8e6a-c4843fed2f9c</rasd:InstanceID> <rasd:ResourceType>32768</rasd:ResourceType> <rasd:VirtualQuantity>1</rasd:VirtualQuantity> <Type>video</Type> <Device>vga</Device> </Item>
great, thank you! Laszlo
Hi Liran, On 11/23/21 17:16, Liran Rotenberg wrote:
On Tue, Nov 23, 2021 at 2:36 PM Laszlo Ersek <lersek@redhat.com> wrote:
Based on feedback from Liran and Arik, here:
https://bugzilla.redhat.com/show_bug.cgi?id=1976607#c3 https://bugzilla.redhat.com/show_bug.cgi?id=1976607#c4
I should use
<Device>vga</Device>
not
<rasd:Device>vga</rasd:Device>
and then ovirt-engine already has code to handle that.
Liran: in comment 3 on the above BZ, you provide the snippet
<Item> <rasd:Caption>Graphical Controller</rasd:Caption> <rasd:InstanceID>2834786b-e9d0-4419-8e6a-c4843fed2f9c</rasd:InstanceID> <rasd:ResourceType>32768</rasd:ResourceType> <rasd:VirtualQuantity>1</rasd:VirtualQuantity> <rasd:SinglePciQxl>false</rasd:SinglePciQxl> <Type>video</Type> <Device>vga</Device> <rasd:Address></rasd:Address> <BootOrder>0</BootOrder> <IsPlugged>true</IsPlugged> <IsReadOnly>false</IsReadOnly> <Alias></Alias> <SpecParams> <vram>16384</vram> </SpecParams> </Item>
From these, virt-v2v does not provide the following at all: - rasd:SinglePciQxl - rasd:Address - BootOrder - IsPlugged - IsReadOnly - Alias - SpecParams (with child node "vram")
Are any of these required, in addition to <Device>vga</Device>?
It will auto set some of the values on import. For being extra careful I checked a VM that I exported omitting these values from the VGA item and it got imported back well(+ran). This was the snippet for the device: <Item> <rasd:Caption>Graphical Controller</rasd:Caption> <rasd:InstanceID>2834786b-e9d0-4419-8e6a-c4843fed2f9c</rasd:InstanceID> <rasd:ResourceType>32768</rasd:ResourceType> <rasd:VirtualQuantity>1</rasd:VirtualQuantity> <Type>video</Type> <Device>vga</Device> </Item>
this XML fragment doesn't seem to have the intended effect. After I updated this patch as discussed: https://bugzilla.redhat.com/show_bug.cgi?id=1961107#c33 <Item> <rasd:Caption>Graphical Controller</rasd:Caption> <rasd:InstanceId>10bc4924-6875-4c18-89d3-8263d589abd1</rasd:InstanceId> <rasd:ResourceType>32768</rasd:ResourceType> <Type>video</Type> <rasd:VirtualQuantity>1</rasd:VirtualQuantity> <Device>vga</Device> </Item> Ming Xie tested it, with rhv-4.4, and the following error was reported: https://bugzilla.redhat.com/show_bug.cgi?id=1961107#c32 --> 1.2 Check guest on rhv after v2v conversion, found video type of guest still shows qxl on rhv4.4, try to power on guest but guest can't power on with error "VM esx7.0-rhel8.5-x86_64 is down with error. Exit message: unsupported configuration: ram attribute only supported for video type qxl." 1.3 Check the video type of guest in vdsm.log, the video type is vga, so need to fix the ram attribute problem for vga type <video> <model type="vga" vram="32768" heads="1" ram="65536" vgamem="16384"></model> <alias name="ua-67a23513-ff93-4663-bae8-5330116642b9"></alias> </video> This patch (after the update) does not generate any video RAM related attributes, so I think those must be ovirt-engine defaults. For reference, the libvirt patch in this series ("output: handle Standard_VGA in the libvirt XML and QEMU cmdline formats") generates the following libvirt XML, for the libvirt output mode: <video> <model type='vga' vram='16384' heads='1'/> </video> (It was successfully tested by Ming Xie in <https://bugzilla.redhat.com/show_bug.cgi?id=1961107#c24>.) Thanks, Laszlo
Removing the "QXL" constructor only needs the obvious fixup in the various pattern matches. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v1: - Drop the "convert/convert_linux.ml" and "convert/windows_virtio.ml" hunks; those files contain no QXL references at this point. - contextual conflict, due to commit f0afc4395248 ("Remove guestcaps_block_type Virtio_SCSI", 2021-11-08), auto-resolved in "lib/types.mli" and "lib/types.ml" - resolve conflict in "output/openstack_image_properties.ml", due to new patch "output: handle Standard_VGA in the OpenStack format" - resolve conflict in "output/create_json.ml", due to new patch "output: handle Standard_VGA in the JSON format" lib/types.ml | 3 +-- lib/types.mli | 2 +- output/create_json.ml | 1 - output/create_libvirt_xml.ml | 1 - output/openstack_image_properties.ml | 1 - output/output.ml | 1 - 6 files changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/types.ml b/lib/types.ml index e7fc8b38787c..864ab8b689ed 100644 --- a/lib/types.ml +++ b/lib/types.ml @@ -424,7 +424,7 @@ type guestcaps = { } and guestcaps_block_type = Virtio_blk | IDE and guestcaps_net_type = Virtio_net | E1000 | RTL8139 -and guestcaps_video_type = Standard_VGA | QXL | Cirrus +and guestcaps_video_type = Standard_VGA | Cirrus and guestcaps_machine = I440FX | Q35 | Virt let string_of_block_type = function @@ -436,7 +436,6 @@ let string_of_net_type = function | RTL8139 -> "rtl8139" let string_of_video = function | Standard_VGA -> "stdvga" - | QXL -> "qxl" | Cirrus -> "cirrus" let string_of_machine = function | I440FX -> "i440fx" diff --git a/lib/types.mli b/lib/types.mli index 3a2ba9fecf9a..a3eef963acb2 100644 --- a/lib/types.mli +++ b/lib/types.mli @@ -284,7 +284,7 @@ type guestcaps = { and guestcaps_block_type = Virtio_blk | IDE and guestcaps_net_type = Virtio_net | E1000 | RTL8139 -and guestcaps_video_type = Standard_VGA | QXL | Cirrus +and guestcaps_video_type = Standard_VGA | Cirrus and guestcaps_machine = I440FX | Q35 | Virt val string_of_guestcaps : guestcaps -> string diff --git a/output/create_json.ml b/output/create_json.ml index ac044630a0c6..add4d886a636 100644 --- a/output/create_json.ml +++ b/output/create_json.ml @@ -204,7 +204,6 @@ let create_json_metadata source inspect let video = match guestcaps.gcaps_video with | Standard_VGA -> "vga" - | QXL -> "qxl" | Cirrus -> "cirrus" in let machine = match guestcaps.gcaps_machine with diff --git a/output/create_libvirt_xml.ml b/output/create_libvirt_xml.ml index 623755de7212..9cf872861110 100644 --- a/output/create_libvirt_xml.ml +++ b/output/create_libvirt_xml.ml @@ -422,7 +422,6 @@ let create_libvirt_xml ?pool source inspect let video_model = match guestcaps.gcaps_video with | Standard_VGA -> e "model" [ "type", "vga"; "vram", "16384" ] [] - | QXL -> e "model" [ "type", "qxl"; "ram", "65536" ] [] | Cirrus -> e "model" [ "type", "cirrus"; "vram", "9216" ] [] in append_attr ("heads", "1") video_model; e "video" [] [ video_model ] in diff --git a/output/openstack_image_properties.ml b/output/openstack_image_properties.ml index 9d466ecfbf0f..2d62efa1a3dc 100644 --- a/output/openstack_image_properties.ml +++ b/output/openstack_image_properties.ml @@ -44,7 +44,6 @@ let create source inspect { target_buses; guestcaps; target_firmware } = "hw_video_model", (match guestcaps.gcaps_video with | Standard_VGA -> "vga" - | QXL -> "qxl" | Cirrus -> "cirrus"); "hw_machine_type", (match guestcaps.gcaps_machine with diff --git a/output/output.ml b/output/output.ml index b27d53059712..4218354f8330 100644 --- a/output/output.ml +++ b/output/output.ml @@ -1542,7 +1542,6 @@ and qemu_finalize dir source inspect target_meta (match guestcaps.gcaps_video with | Standard_VGA -> "std" | Cirrus -> "cirrus" - | QXL -> "qxl" ) ); -- 2.19.1.3.g30247aa5d201
The QXL drivers no longer matter when converting Windows guests; remove the fake images and their references from the test suite. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v1: - no change test-data/fake-virtio-win/Makefile.am | 16 ---------------- test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.inf | 2 -- test-data/fake-virtio-win/drivers/amd64/Win7/qxl.inf | 2 -- test-data/fake-virtio-win/drivers/i386/Win7/qxl.inf | 2 -- test-data/fake-virtio-win/drivers/i386/WinXP/qxl.inf | 2 -- test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.cat | 1 - test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.sys | 1 - test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxldd.dll | 1 - test-data/fake-virtio-win/drivers/amd64/Win7/qxl.cat | 1 - test-data/fake-virtio-win/drivers/amd64/Win7/qxl.sys | 1 - test-data/fake-virtio-win/drivers/amd64/Win7/qxldd.dll | 1 - test-data/fake-virtio-win/drivers/i386/Win7/qxl.cat | 1 - test-data/fake-virtio-win/drivers/i386/Win7/qxl.sys | 1 - test-data/fake-virtio-win/drivers/i386/Win7/qxldd.dll | 1 - test-data/fake-virtio-win/drivers/i386/WinXP/qxl.cat | 1 - test-data/fake-virtio-win/drivers/i386/WinXP/qxl.sys | 1 - test-data/fake-virtio-win/drivers/i386/WinXP/qxldd.dll | 1 - tests/test-v2v-in-place.sh | 2 +- tests/test-v2v-windows-conversion.sh | 2 +- v2v/v2v_unit_tests.ml | 16 ---------------- 20 files changed, 2 insertions(+), 54 deletions(-) diff --git a/test-data/fake-virtio-win/Makefile.am b/test-data/fake-virtio-win/Makefile.am index 55b91a95a5e5..60c2d8245db9 100644 --- a/test-data/fake-virtio-win/Makefile.am +++ b/test-data/fake-virtio-win/Makefile.am @@ -38,10 +38,6 @@ drivers = \ drivers/amd64/Win2008R2/netkvm.cat \ drivers/amd64/Win2008R2/netkvm.inf \ drivers/amd64/Win2008R2/netkvm.sys \ - drivers/amd64/Win2008R2/qxl.cat \ - drivers/amd64/Win2008R2/qxldd.dll \ - drivers/amd64/Win2008R2/qxl.inf \ - drivers/amd64/Win2008R2/qxl.sys \ drivers/amd64/Win2008R2/vioscsi.cat \ drivers/amd64/Win2008R2/vioscsi.inf \ drivers/amd64/Win2008R2/vioscsi.sys \ @@ -75,10 +71,6 @@ drivers = \ drivers/amd64/Win7/netkvm.cat \ drivers/amd64/Win7/netkvm.inf \ drivers/amd64/Win7/netkvm.sys \ - drivers/amd64/Win7/qxl.cat \ - drivers/amd64/Win7/qxldd.dll \ - drivers/amd64/Win7/qxl.inf \ - drivers/amd64/Win7/qxl.sys \ drivers/amd64/Win7/vioscsi.cat \ drivers/amd64/Win7/vioscsi.inf \ drivers/amd64/Win7/vioscsi.sys \ @@ -121,10 +113,6 @@ drivers = \ drivers/i386/Win7/netkvm.cat \ drivers/i386/Win7/netkvm.inf \ drivers/i386/Win7/netkvm.sys \ - drivers/i386/Win7/qxl.cat \ - drivers/i386/Win7/qxldd.dll \ - drivers/i386/Win7/qxl.inf \ - drivers/i386/Win7/qxl.sys \ drivers/i386/Win7/vioscsi.cat \ drivers/i386/Win7/vioscsi.inf \ drivers/i386/Win7/vioscsi.sys \ @@ -152,10 +140,6 @@ drivers = \ drivers/i386/WinXP/netkvm.cat \ drivers/i386/WinXP/netkvm.inf \ drivers/i386/WinXP/netkvm.sys \ - drivers/i386/WinXP/qxl.cat \ - drivers/i386/WinXP/qxldd.dll \ - drivers/i386/WinXP/qxl.inf \ - drivers/i386/WinXP/qxl.sys \ drivers/i386/WinXP/viostor.cat \ drivers/i386/WinXP/viostor.inf \ drivers/i386/WinXP/viostor.sys diff --git a/test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.inf b/test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.inf deleted file mode 100644 index 67e3fab0f3a7..000000000000 --- a/test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.inf +++ /dev/null @@ -1,2 +0,0 @@ -[Version] -DriverVer = 10/29/2013,6.1.0.10020 diff --git a/test-data/fake-virtio-win/drivers/amd64/Win7/qxl.inf b/test-data/fake-virtio-win/drivers/amd64/Win7/qxl.inf deleted file mode 100644 index 3310167e395f..000000000000 --- a/test-data/fake-virtio-win/drivers/amd64/Win7/qxl.inf +++ /dev/null @@ -1,2 +0,0 @@ -[Version] -DriverVer = 07/17/2013,6.1.0.10018 diff --git a/test-data/fake-virtio-win/drivers/i386/Win7/qxl.inf b/test-data/fake-virtio-win/drivers/i386/Win7/qxl.inf deleted file mode 100644 index 3310167e395f..000000000000 --- a/test-data/fake-virtio-win/drivers/i386/Win7/qxl.inf +++ /dev/null @@ -1,2 +0,0 @@ -[Version] -DriverVer = 07/17/2013,6.1.0.10018 diff --git a/test-data/fake-virtio-win/drivers/i386/WinXP/qxl.inf b/test-data/fake-virtio-win/drivers/i386/WinXP/qxl.inf deleted file mode 100644 index d1fe96b24e79..000000000000 --- a/test-data/fake-virtio-win/drivers/i386/WinXP/qxl.inf +++ /dev/null @@ -1,2 +0,0 @@ -[Version] -DriverVer = 07/17/2013,5.1.0.10018 diff --git a/test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.cat b/test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.cat deleted file mode 100644 index acbc962012bc..000000000000 --- a/test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.cat +++ /dev/null @@ -1 +0,0 @@ -fake diff --git a/test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.sys b/test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.sys deleted file mode 100644 index acbc962012bc..000000000000 --- a/test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxl.sys +++ /dev/null @@ -1 +0,0 @@ -fake diff --git a/test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxldd.dll b/test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxldd.dll deleted file mode 100644 index acbc962012bc..000000000000 --- a/test-data/fake-virtio-win/drivers/amd64/Win2008R2/qxldd.dll +++ /dev/null @@ -1 +0,0 @@ -fake diff --git a/test-data/fake-virtio-win/drivers/amd64/Win7/qxl.cat b/test-data/fake-virtio-win/drivers/amd64/Win7/qxl.cat deleted file mode 100644 index acbc962012bc..000000000000 --- a/test-data/fake-virtio-win/drivers/amd64/Win7/qxl.cat +++ /dev/null @@ -1 +0,0 @@ -fake diff --git a/test-data/fake-virtio-win/drivers/amd64/Win7/qxl.sys b/test-data/fake-virtio-win/drivers/amd64/Win7/qxl.sys deleted file mode 100644 index acbc962012bc..000000000000 --- a/test-data/fake-virtio-win/drivers/amd64/Win7/qxl.sys +++ /dev/null @@ -1 +0,0 @@ -fake diff --git a/test-data/fake-virtio-win/drivers/amd64/Win7/qxldd.dll b/test-data/fake-virtio-win/drivers/amd64/Win7/qxldd.dll deleted file mode 100644 index acbc962012bc..000000000000 --- a/test-data/fake-virtio-win/drivers/amd64/Win7/qxldd.dll +++ /dev/null @@ -1 +0,0 @@ -fake diff --git a/test-data/fake-virtio-win/drivers/i386/Win7/qxl.cat b/test-data/fake-virtio-win/drivers/i386/Win7/qxl.cat deleted file mode 100644 index acbc962012bc..000000000000 --- a/test-data/fake-virtio-win/drivers/i386/Win7/qxl.cat +++ /dev/null @@ -1 +0,0 @@ -fake diff --git a/test-data/fake-virtio-win/drivers/i386/Win7/qxl.sys b/test-data/fake-virtio-win/drivers/i386/Win7/qxl.sys deleted file mode 100644 index acbc962012bc..000000000000 --- a/test-data/fake-virtio-win/drivers/i386/Win7/qxl.sys +++ /dev/null @@ -1 +0,0 @@ -fake diff --git a/test-data/fake-virtio-win/drivers/i386/Win7/qxldd.dll b/test-data/fake-virtio-win/drivers/i386/Win7/qxldd.dll deleted file mode 100644 index acbc962012bc..000000000000 --- a/test-data/fake-virtio-win/drivers/i386/Win7/qxldd.dll +++ /dev/null @@ -1 +0,0 @@ -fake diff --git a/test-data/fake-virtio-win/drivers/i386/WinXP/qxl.cat b/test-data/fake-virtio-win/drivers/i386/WinXP/qxl.cat deleted file mode 100644 index acbc962012bc..000000000000 --- a/test-data/fake-virtio-win/drivers/i386/WinXP/qxl.cat +++ /dev/null @@ -1 +0,0 @@ -fake diff --git a/test-data/fake-virtio-win/drivers/i386/WinXP/qxl.sys b/test-data/fake-virtio-win/drivers/i386/WinXP/qxl.sys deleted file mode 100644 index acbc962012bc..000000000000 --- a/test-data/fake-virtio-win/drivers/i386/WinXP/qxl.sys +++ /dev/null @@ -1 +0,0 @@ -fake diff --git a/test-data/fake-virtio-win/drivers/i386/WinXP/qxldd.dll b/test-data/fake-virtio-win/drivers/i386/WinXP/qxldd.dll deleted file mode 100644 index acbc962012bc..000000000000 --- a/test-data/fake-virtio-win/drivers/i386/WinXP/qxldd.dll +++ /dev/null @@ -1 +0,0 @@ -fake diff --git a/tests/test-v2v-in-place.sh b/tests/test-v2v-in-place.sh index 9da8fa63a99b..c5fee2f9f591 100755 --- a/tests/test-v2v-in-place.sh +++ b/tests/test-v2v-in-place.sh @@ -95,7 +95,7 @@ mktest "is-file \"$firstboot_dir/firstboot.bat\"" true mktest "is-dir \"$firstboot_dir/scripts\"" true virtio_dir="/Windows/Drivers/VirtIO" mktest "is-dir \"$virtio_dir\"" true -for drv in netkvm qxl vioscsi viostor; do +for drv in netkvm vioscsi viostor; do for sfx in cat inf sys; do mktest "is-file \"$virtio_dir/$drv.$sfx\"" true done diff --git a/tests/test-v2v-windows-conversion.sh b/tests/test-v2v-windows-conversion.sh index aeab9a48b507..a4cf191d74b4 100755 --- a/tests/test-v2v-windows-conversion.sh +++ b/tests/test-v2v-windows-conversion.sh @@ -82,7 +82,7 @@ mktest "is-file \"$firstboot_dir/firstboot.bat\"" true mktest "is-dir \"$firstboot_dir/scripts\"" true virtio_dir="/Windows/Drivers/VirtIO" mktest "is-dir \"$virtio_dir\"" true -for drv in netkvm qxl vioscsi viostor; do +for drv in netkvm vioscsi viostor; do for sfx in cat inf sys; do mktest "is-file \"$virtio_dir/$drv.$sfx\"" true done diff --git a/v2v/v2v_unit_tests.ml b/v2v/v2v_unit_tests.ml index 584c5bb7b395..889f7998cf3b 100644 --- a/v2v/v2v_unit_tests.ml +++ b/v2v/v2v_unit_tests.ml @@ -603,15 +603,11 @@ let test_virtio_iso_path_matches_guest_os ctx = "drivers/i386/Win2008/vioscsi.inf", Some win2k8_32; "drivers/i386/Win7/viostor.inf", Some win7_32; "drivers/i386/Win7/viostor.sys", Some win7_32; - "drivers/i386/Win7/qxldd.dll", Some win7_32; - "drivers/i386/Win7/qxl.sys", Some win7_32; "drivers/i386/Win7/vioscsi.cat", Some win7_32; "drivers/i386/Win7/netkvm.inf", Some win7_32; "drivers/i386/Win7/netkvm.sys", Some win7_32; "drivers/i386/Win7/viostor.cat", Some win7_32; - "drivers/i386/Win7/qxl.inf", Some win7_32; "drivers/i386/Win7/vioscsi.sys", Some win7_32; - "drivers/i386/Win7/qxl.cat", Some win7_32; "drivers/i386/Win7/netkvm.cat", Some win7_32; "drivers/i386/Win7/vioscsi.inf", Some win7_32; "drivers/i386/Win2003/viostor.inf", Some win2k3_32; @@ -631,13 +627,9 @@ let test_virtio_iso_path_matches_guest_os ctx = "drivers/i386/Win8/vioscsi.inf", Some win8_32; "drivers/i386/WinXP/viostor.inf", Some winxp_32; "drivers/i386/WinXP/viostor.sys", Some winxp_32; - "drivers/i386/WinXP/qxldd.dll", Some winxp_32; - "drivers/i386/WinXP/qxl.sys", Some winxp_32; "drivers/i386/WinXP/netkvm.inf", Some winxp_32; "drivers/i386/WinXP/netkvm.sys", Some winxp_32; "drivers/i386/WinXP/viostor.cat", Some winxp_32; - "drivers/i386/WinXP/qxl.inf", Some winxp_32; - "drivers/i386/WinXP/qxl.cat", Some winxp_32; "drivers/i386/WinXP/netkvm.cat", Some winxp_32; "drivers/amd64/Win8.1/viostor.inf", Some win8_1_64; "drivers/amd64/Win8.1/viostor.sys", Some win8_1_64; @@ -659,15 +651,11 @@ let test_virtio_iso_path_matches_guest_os ctx = "drivers/amd64/Win2008/vioscsi.inf", Some win2k8_64; "drivers/amd64/Win7/viostor.inf", Some win7_64; "drivers/amd64/Win7/viostor.sys", Some win7_64; - "drivers/amd64/Win7/qxldd.dll", Some win7_64; - "drivers/amd64/Win7/qxl.sys", Some win7_64; "drivers/amd64/Win7/vioscsi.cat", Some win7_64; "drivers/amd64/Win7/netkvm.inf", Some win7_64; "drivers/amd64/Win7/netkvm.sys", Some win7_64; "drivers/amd64/Win7/viostor.cat", Some win7_64; - "drivers/amd64/Win7/qxl.inf", Some win7_64; "drivers/amd64/Win7/vioscsi.sys", Some win7_64; - "drivers/amd64/Win7/qxl.cat", Some win7_64; "drivers/amd64/Win7/netkvm.cat", Some win7_64; "drivers/amd64/Win7/vioscsi.inf", Some win7_64; "drivers/amd64/Win2003/viostor.inf", Some win2k3_64; @@ -696,15 +684,11 @@ let test_virtio_iso_path_matches_guest_os ctx = "drivers/amd64/Win2012/vioscsi.inf", Some win2k12_64; "drivers/amd64/Win2008R2/viostor.inf", Some win2k8r2_64; "drivers/amd64/Win2008R2/viostor.sys", Some win2k8r2_64; - "drivers/amd64/Win2008R2/qxldd.dll", Some win2k8r2_64; - "drivers/amd64/Win2008R2/qxl.sys", Some win2k8r2_64; "drivers/amd64/Win2008R2/vioscsi.cat", Some win2k8r2_64; "drivers/amd64/Win2008R2/netkvm.inf", Some win2k8r2_64; "drivers/amd64/Win2008R2/netkvm.sys", Some win2k8r2_64; "drivers/amd64/Win2008R2/viostor.cat", Some win2k8r2_64; - "drivers/amd64/Win2008R2/qxl.inf", Some win2k8r2_64; "drivers/amd64/Win2008R2/vioscsi.sys", Some win2k8r2_64; - "drivers/amd64/Win2008R2/qxl.cat", Some win2k8r2_64; "drivers/amd64/Win2008R2/netkvm.cat", Some win2k8r2_64; "drivers/amd64/Win2008R2/vioscsi.inf", Some win2k8r2_64; "drivers/amd64/Win2012R2/viostor.inf", Some win2k12r2_64; -- 2.19.1.3.g30247aa5d201
Just for completeness' sake, change the display device from QXL to standard VGA in: - test-data/phony-guests/guests.xml.in - tests/test-v2v-print-source.xml.in This removes the last QXL occurrences in the source files. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v1: - no change test-data/phony-guests/guests.xml.in | 2 +- tests/test-v2v-print-source.expected | 2 +- tests/test-v2v-print-source.xml.in | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test-data/phony-guests/guests.xml.in b/test-data/phony-guests/guests.xml.in index 4139d04f689f..9f64c35cd80f 100644 --- a/test-data/phony-guests/guests.xml.in +++ b/test-data/phony-guests/guests.xml.in @@ -299,7 +299,7 @@ <model type='virtio'/> </interface> <video> - <model type='qxl' ram='65536' vram='65536' vgamem='16384' heads='1'/> + <model type='vga' vram='16384' heads='1'/> </video> </devices> </domain> diff --git a/tests/test-v2v-print-source.expected b/tests/test-v2v-print-source.expected index afed30b5c46d..cfeade1c012d 100644 --- a/tests/test-v2v-print-source.expected +++ b/tests/test-v2v-print-source.expected @@ -9,7 +9,7 @@ hypervisor type: kvm CPU features: acpi,apic,pae firmware: unknown display: - video: qxl + video: vga sound: disks: 0 [virtio-blk] diff --git a/tests/test-v2v-print-source.xml.in b/tests/test-v2v-print-source.xml.in index 2f83fc73cb8a..d018d5fbcd27 100644 --- a/tests/test-v2v-print-source.xml.in +++ b/tests/test-v2v-print-source.xml.in @@ -27,7 +27,7 @@ <model type='virtio'/> </interface> <video> - <model type='qxl' ram='65536' vram='65536' vgamem='16384' heads='1'/> + <model type='vga' vram='16384' heads='1'/> </video> </devices> </domain> -- 2.19.1.3.g30247aa5d201
At this point, virt-v2v only recognizes, but never creates, the "Cirrus" value of "guestcaps_video_type". Remove "Cirrus", and simplify the affected pattern matches. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v1: - new in v1 lib/types.ml | 3 +-- lib/types.mli | 2 +- output/create_json.ml | 3 +-- output/create_libvirt_xml.ml | 3 +-- output/openstack_image_properties.ml | 3 +-- output/output.ml | 1 - 6 files changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/types.ml b/lib/types.ml index 864ab8b689ed..37696ebbabdd 100644 --- a/lib/types.ml +++ b/lib/types.ml @@ -424,7 +424,7 @@ type guestcaps = { } and guestcaps_block_type = Virtio_blk | IDE and guestcaps_net_type = Virtio_net | E1000 | RTL8139 -and guestcaps_video_type = Standard_VGA | Cirrus +and guestcaps_video_type = Standard_VGA and guestcaps_machine = I440FX | Q35 | Virt let string_of_block_type = function @@ -436,7 +436,6 @@ let string_of_net_type = function | RTL8139 -> "rtl8139" let string_of_video = function | Standard_VGA -> "stdvga" - | Cirrus -> "cirrus" let string_of_machine = function | I440FX -> "i440fx" | Q35 -> "q35" diff --git a/lib/types.mli b/lib/types.mli index a3eef963acb2..8d48c300ec17 100644 --- a/lib/types.mli +++ b/lib/types.mli @@ -284,7 +284,7 @@ type guestcaps = { and guestcaps_block_type = Virtio_blk | IDE and guestcaps_net_type = Virtio_net | E1000 | RTL8139 -and guestcaps_video_type = Standard_VGA | Cirrus +and guestcaps_video_type = Standard_VGA and guestcaps_machine = I440FX | Q35 | Virt val string_of_guestcaps : guestcaps -> string diff --git a/output/create_json.ml b/output/create_json.ml index add4d886a636..3f6a734c9f8e 100644 --- a/output/create_json.ml +++ b/output/create_json.ml @@ -203,8 +203,7 @@ let create_json_metadata source inspect | RTL8139 -> "rtl8139" in let video = match guestcaps.gcaps_video with - | Standard_VGA -> "vga" - | Cirrus -> "cirrus" in + | Standard_VGA -> "vga" in let machine = match guestcaps.gcaps_machine with | I440FX -> "i440fx" diff --git a/output/create_libvirt_xml.ml b/output/create_libvirt_xml.ml index 9cf872861110..041643969c23 100644 --- a/output/create_libvirt_xml.ml +++ b/output/create_libvirt_xml.ml @@ -421,8 +421,7 @@ let create_libvirt_xml ?pool source inspect let video = let video_model = match guestcaps.gcaps_video with - | Standard_VGA -> e "model" [ "type", "vga"; "vram", "16384" ] [] - | Cirrus -> e "model" [ "type", "cirrus"; "vram", "9216" ] [] in + | Standard_VGA -> e "model" [ "type", "vga"; "vram", "16384" ] [] in append_attr ("heads", "1") video_model; e "video" [] [ video_model ] in List.push_back devices video; diff --git a/output/openstack_image_properties.ml b/output/openstack_image_properties.ml index 2d62efa1a3dc..0a7d366967b6 100644 --- a/output/openstack_image_properties.ml +++ b/output/openstack_image_properties.ml @@ -43,8 +43,7 @@ let create source inspect { target_buses; guestcaps; target_firmware } = | RTL8139 -> "rtl8139"); "hw_video_model", (match guestcaps.gcaps_video with - | Standard_VGA -> "vga" - | Cirrus -> "cirrus"); + | Standard_VGA -> "vga"); "hw_machine_type", (match guestcaps.gcaps_machine with | I440FX -> "pc" diff --git a/output/output.ml b/output/output.ml index 4218354f8330..340883d97717 100644 --- a/output/output.ml +++ b/output/output.ml @@ -1541,7 +1541,6 @@ and qemu_finalize dir source inspect target_meta arg "-vga" (match guestcaps.gcaps_video with | Standard_VGA -> "std" - | Cirrus -> "cirrus" ) ); -- 2.19.1.3.g30247aa5d201
When the "virtio_socket_supported" component was added to the "install_drivers" return tuple, the documentation was not updated. Do it now. Fixes: 05f780c16f0135c657615520c2245b42de1efc3e Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v1: - new in v1 convert/windows_virtio.mli | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/convert/windows_virtio.mli b/convert/windows_virtio.mli index 4e24625a4edf..b314d01da6cb 100644 --- a/convert/windows_virtio.mli +++ b/convert/windows_virtio.mli @@ -30,10 +30,10 @@ val install_drivers function is called. This returns the tuple [(block_driver, net_driver, video_driver, - virtio_rng_supported, virtio_ballon_supported, isa_pvpanic_supported)] - reflecting what devices are now required by the guest, either - virtio devices if we managed to install those, or legacy devices - if we didn't. *) + virtio_rng_supported, virtio_ballon_supported, isa_pvpanic_supported, + virtio_socket_supported)] reflecting what devices are now required by the + guest, either virtio devices if we managed to install those, or legacy + devices if we didn't. *) val install_linux_tools : Guestfs.guestfs -> Types.inspect -> unit (** installs QEMU Guest Agent on Linux guest OS from the driver directory or -- 2.19.1.3.g30247aa5d201
The "guestcaps_video_type" variant type now has a single (non-parametric) constructor (namely "Standard_VGA"). Replace all uses of this type / constructor with (simpler) constant expressions, wherever values are necessary. Remove "guestcaps_video_type" from dependent types, where it effectively no longer carries any information. (The "string_of_video" function is only used for debugging, via "string_of_guestcaps", in "do_convert" [convert/convert.ml]). Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v1: - new in v1 convert/convert_linux.ml | 1 - convert/convert_windows.ml | 2 -- convert/windows_virtio.ml | 4 ++-- convert/windows_virtio.mli | 11 +++++------ lib/types.ml | 6 ------ lib/types.mli | 11 ++++------- output/create_json.ml | 4 +--- output/create_libvirt_xml.ml | 3 +-- output/openstack_image_properties.ml | 4 +--- output/output.ml | 5 +---- 10 files changed, 15 insertions(+), 36 deletions(-) diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml index f0213f06eec2..8dc648169dcb 100644 --- a/convert/convert_linux.ml +++ b/convert/convert_linux.ml @@ -148,7 +148,6 @@ let convert (g : G.guestfs) source inspect keep_serial_console _ = let guestcaps = { gcaps_block_bus = block_type; gcaps_net_bus = net_type; - gcaps_video = Standard_VGA; gcaps_virtio_rng = kernel.ki_supports_virtio_rng; gcaps_virtio_balloon = kernel.ki_supports_virtio_balloon; gcaps_isa_pvpanic = kernel.ki_supports_isa_pvpanic; diff --git a/convert/convert_windows.ml b/convert/convert_windows.ml index 1a653b88e7fa..0ebffb156633 100644 --- a/convert/convert_windows.ml +++ b/convert/convert_windows.ml @@ -211,7 +211,6 @@ let convert (g : G.guestfs) _ inspect _ static_ips = (* Open the system hive for writes and update it. *) let block_driver, net_driver, - video_driver, virtio_rng_supported, virtio_ballon_supported, isa_pvpanic_supported, @@ -253,7 +252,6 @@ let convert (g : G.guestfs) _ inspect _ static_ips = let guestcaps = { gcaps_block_bus = block_driver; gcaps_net_bus = net_driver; - gcaps_video = video_driver; gcaps_virtio_rng = virtio_rng_supported; gcaps_virtio_balloon = virtio_ballon_supported; gcaps_isa_pvpanic = isa_pvpanic_supported; diff --git a/convert/windows_virtio.ml b/convert/windows_virtio.ml index 77aeac2d4a9f..1c5c148e65fd 100644 --- a/convert/windows_virtio.ml +++ b/convert/windows_virtio.ml @@ -53,7 +53,7 @@ let rec install_drivers ((g, _) as reg) inspect = warning (f_"there are no virtio drivers available for this version of Windows (%d.%d %s %s). virt-v2v looks for drivers in %s\n\nThe guest will be configured to use slower emulated devices.") inspect.i_major_version inspect.i_minor_version inspect.i_arch inspect.i_product_variant virtio_win; - (IDE, RTL8139, Standard_VGA, false, false, false, false) + (IDE, RTL8139, false, false, false, false) ) else ( (* Can we install the block driver? *) @@ -109,7 +109,7 @@ let rec install_drivers ((g, _) as reg) inspect = let isa_pvpanic_supported = g#exists (driverdir // "pvpanic.inf") in let virtio_socket_supported = g#exists (driverdir // "viosock.inf") in - (block, net, Standard_VGA, + (block, net, virtio_rng_supported, virtio_ballon_supported, isa_pvpanic_supported, virtio_socket_supported) ) diff --git a/convert/windows_virtio.mli b/convert/windows_virtio.mli index b314d01da6cb..53603d240553 100644 --- a/convert/windows_virtio.mli +++ b/convert/windows_virtio.mli @@ -20,7 +20,7 @@ val install_drivers : Registry.t -> Types.inspect -> - Types.guestcaps_block_type * Types.guestcaps_net_type * Types.guestcaps_video_type * bool * bool * bool * bool + Types.guestcaps_block_type * Types.guestcaps_net_type * bool * bool * bool * bool (** [install_drivers reg inspect] installs virtio drivers from the driver directory or driver ISO into the guest driver directory and updates the registry @@ -29,11 +29,10 @@ val install_drivers [reg] is the system hive which is open for writes when this function is called. - This returns the tuple [(block_driver, net_driver, video_driver, - virtio_rng_supported, virtio_ballon_supported, isa_pvpanic_supported, - virtio_socket_supported)] reflecting what devices are now required by the - guest, either virtio devices if we managed to install those, or legacy - devices if we didn't. *) + This returns the tuple [(block_driver, net_driver, virtio_rng_supported, + virtio_ballon_supported, isa_pvpanic_supported, virtio_socket_supported)] + reflecting what devices are now required by the guest, either virtio + devices if we managed to install those, or legacy devices if we didn't. *) val install_linux_tools : Guestfs.guestfs -> Types.inspect -> unit (** installs QEMU Guest Agent on Linux guest OS from the driver directory or diff --git a/lib/types.ml b/lib/types.ml index 37696ebbabdd..acca0d989b91 100644 --- a/lib/types.ml +++ b/lib/types.ml @@ -413,7 +413,6 @@ type target_nics = source_nic list type guestcaps = { gcaps_block_bus : guestcaps_block_type; gcaps_net_bus : guestcaps_net_type; - gcaps_video : guestcaps_video_type; gcaps_virtio_rng : bool; gcaps_virtio_balloon : bool; gcaps_isa_pvpanic : bool; @@ -424,7 +423,6 @@ type guestcaps = { } and guestcaps_block_type = Virtio_blk | IDE and guestcaps_net_type = Virtio_net | E1000 | RTL8139 -and guestcaps_video_type = Standard_VGA and guestcaps_machine = I440FX | Q35 | Virt let string_of_block_type = function @@ -434,8 +432,6 @@ let string_of_net_type = function | Virtio_net -> "virtio-net" | E1000 -> "e1000" | RTL8139 -> "rtl8139" -let string_of_video = function - | Standard_VGA -> "stdvga" let string_of_machine = function | I440FX -> "i440fx" | Q35 -> "q35" @@ -445,13 +441,11 @@ let string_of_guestcaps gcaps = sprintf "\ gcaps_block_bus = %s gcaps_net_bus = %s -gcaps_video = %s gcaps_machine = %s gcaps_arch = %s gcaps_acpi = %b " (string_of_block_type gcaps.gcaps_block_bus) (string_of_net_type gcaps.gcaps_net_bus) - (string_of_video gcaps.gcaps_video) (string_of_machine gcaps.gcaps_machine) gcaps.gcaps_arch gcaps.gcaps_acpi diff --git a/lib/types.mli b/lib/types.mli index 8d48c300ec17..e0ef222e0455 100644 --- a/lib/types.mli +++ b/lib/types.mli @@ -265,12 +265,10 @@ type target_nics = source_nic list type guestcaps = { gcaps_block_bus : guestcaps_block_type; gcaps_net_bus : guestcaps_net_type; - gcaps_video : guestcaps_video_type; - (** Best block device, network device and video device guest can - access. These are determined during conversion by inspecting the - guest (and in some cases conversion can actually enhance these by - installing drivers). Thus this is not known until after - conversion. *) + (** Best block device and network device guest can access. These are + determined during conversion by inspecting the guest (and in some cases + conversion can actually enhance these by installing drivers). Thus this + is not known until after conversion. *) gcaps_virtio_rng : bool; (** Guest supports virtio-rng. *) gcaps_virtio_balloon : bool; (** Guest supports virtio balloon. *) @@ -284,7 +282,6 @@ type guestcaps = { and guestcaps_block_type = Virtio_blk | IDE and guestcaps_net_type = Virtio_net | E1000 | RTL8139 -and guestcaps_video_type = Standard_VGA and guestcaps_machine = I440FX | Q35 | Virt val string_of_guestcaps : guestcaps -> string diff --git a/output/create_json.ml b/output/create_json.ml index 3f6a734c9f8e..b48902b7d221 100644 --- a/output/create_json.ml +++ b/output/create_json.ml @@ -201,9 +201,7 @@ let create_json_metadata source inspect | Virtio_net -> "virtio-net" | E1000 -> "e1000" | RTL8139 -> "rtl8139" in - let video = - match guestcaps.gcaps_video with - | Standard_VGA -> "vga" in + let video = "vga" in let machine = match guestcaps.gcaps_machine with | I440FX -> "i440fx" diff --git a/output/create_libvirt_xml.ml b/output/create_libvirt_xml.ml index 041643969c23..9413cc0bf884 100644 --- a/output/create_libvirt_xml.ml +++ b/output/create_libvirt_xml.ml @@ -420,8 +420,7 @@ let create_libvirt_xml ?pool source inspect *) let video = let video_model = - match guestcaps.gcaps_video with - | Standard_VGA -> e "model" [ "type", "vga"; "vram", "16384" ] [] in + e "model" [ "type", "vga"; "vram", "16384" ] [] in append_attr ("heads", "1") video_model; e "video" [] [ video_model ] in List.push_back devices video; diff --git a/output/openstack_image_properties.ml b/output/openstack_image_properties.ml index 0a7d366967b6..c75d72fe6942 100644 --- a/output/openstack_image_properties.ml +++ b/output/openstack_image_properties.ml @@ -41,9 +41,7 @@ let create source inspect { target_buses; guestcaps; target_firmware } = | Virtio_net -> "virtio" | E1000 -> "e1000" | RTL8139 -> "rtl8139"); - "hw_video_model", - (match guestcaps.gcaps_video with - | Standard_VGA -> "vga"); + "hw_video_model", "vga"; "hw_machine_type", (match guestcaps.gcaps_machine with | I440FX -> "pc" diff --git a/output/output.ml b/output/output.ml index 340883d97717..41a816108847 100644 --- a/output/output.ml +++ b/output/output.ml @@ -1538,10 +1538,7 @@ and qemu_finalize dir source inspect target_meta | Some p -> p); "addr=127.0.0.1"] ); - arg "-vga" - (match guestcaps.gcaps_video with - | Standard_VGA -> "std" - ) + arg "-vga" "std" ); (* Add a sound card. *) -- 2.19.1.3.g30247aa5d201
The "source_video" union type is basically a string, with a distinguished constructor for the "cirrus" device model ("Source_Cirrus"). However, this distinction has been useless since commit 255722cbf39a ("v2v: Modular virt-v2v", 2021-09-07); now "Source_Cirrus" isn't treated specially. Replace the "source_video" type with just "string". Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v1: - new in v1 input/parse_domain_from_vmx.ml | 2 +- input/parse_libvirt_xml.ml | 4 +--- lib/types.ml | 15 ++------------- lib/types.mli | 8 +------- 4 files changed, 5 insertions(+), 24 deletions(-) diff --git a/input/parse_domain_from_vmx.ml b/input/parse_domain_from_vmx.ml index 4f8beb04a8be..650ddbb7bac1 100644 --- a/input/parse_domain_from_vmx.ml +++ b/input/parse_domain_from_vmx.ml @@ -415,7 +415,7 @@ let parse_domain_from_vmx vmx_source = let video = if Parse_vmx.namespace_present vmx ["svga"] then (* We could also parse svga.vramSize. *) - Some (Source_other_video "vmvga") + Some "vmvga" else None in diff --git a/input/parse_libvirt_xml.ml b/input/parse_libvirt_xml.ml index c53b8001ca77..835649939d22 100644 --- a/input/parse_libvirt_xml.ml +++ b/input/parse_libvirt_xml.ml @@ -193,9 +193,7 @@ let parse_libvirt_xml ?conn xml = let node = Xml.xpathobj_node obj 0 in Xml.xpathctx_set_current_context xpathctx node; - match xpath_string "model/@type" with - | None -> None - | Some model -> Some (source_video_of_string model) + xpath_string "model/@type" ) in (* Sound card. *) diff --git a/lib/types.ml b/lib/types.ml index acca0d989b91..9629bab50d4e 100644 --- a/lib/types.ml +++ b/lib/types.ml @@ -36,7 +36,7 @@ type source = { s_features : string list; s_firmware : source_firmware; s_display : source_display option; - s_video : source_video option; + s_video : string option; s_sound : source_sound option; s_disks : source_disk list; s_removables : source_removable list; @@ -89,9 +89,6 @@ and s_display_listen = | LSocket of string option | LNone -and source_video = Source_other_video of string | - Source_Cirrus - and source_sound = { s_sound_model : source_sound_model; } @@ -141,7 +138,7 @@ NICs: | Some display -> string_of_source_display display) (match s.s_video with | None -> "" - | Some video -> string_of_source_video video) + | Some video -> video) (match s.s_sound with | None -> "" | Some sound -> string_of_source_sound sound) @@ -259,14 +256,6 @@ and string_of_source_display { s_display_type = typ; | LNone -> " listening on private fd" ) -and string_of_source_video = function - | Source_Cirrus -> "cirrus" - | Source_other_video video -> video - -and source_video_of_string = function - | "cirrus" -> Source_Cirrus - | video -> Source_other_video video - and string_of_source_sound { s_sound_model = model } = string_of_source_sound_model model diff --git a/lib/types.mli b/lib/types.mli index e0ef222e0455..65efbcc03fec 100644 --- a/lib/types.mli +++ b/lib/types.mli @@ -70,7 +70,7 @@ type source = { s_features : string list; (** Machine features. *) s_firmware : source_firmware; (** Firmware (BIOS or EFI). *) s_display : source_display option; (** Guest display. *) - s_video : source_video option; (** Video adapter. *) + s_video : string option; (** Video adapter. *) s_sound : source_sound option; (** Sound card. *) s_disks : source_disk list; (** Source disks. *) s_removables : source_removable list; (** CDROMs etc. *) @@ -145,10 +145,6 @@ and s_display_listen = | LSocket of string option (** Listen Unix domain socket. *) | LNone (** <listen type='none'> *) -(** Video adapter model. *) -and source_video = Source_other_video of string | - Source_Cirrus - and source_sound = { s_sound_model : source_sound_model; (** Sound model. *) } @@ -169,8 +165,6 @@ val nic_model_of_string : string -> s_nic_model val string_of_vnet_type : vnet_type -> string val string_of_source_sound_model : source_sound_model -> string val source_sound_model_of_string : string -> source_sound_model option -val string_of_source_video : source_video -> string -val source_video_of_string : string -> source_video val string_of_source_cpu_topology : source_cpu_topology -> string val string_of_source_hypervisor : source_hypervisor -> string -- 2.19.1.3.g30247aa5d201
Since commit 255722cbf39a ("v2v: Modular virt-v2v", 2021-09-07), the only use of the "source.s_video" field has been its logging via "string_of_source", for the "--print-source" debugging option. Given that the source video controller is available in the source domain description anyway (that's where "source.s_video" is parsed from), simplify the debug log code by removing "source.s_video". Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v1: - new in v1 input/input.ml | 2 -- input/parse_domain_from_vmx.ml | 8 -------- input/parse_libvirt_xml.ml | 14 -------------- lib/types.ml | 5 ----- lib/types.mli | 1 - tests/test-v2v-i-ova-formats.expected | 1 - tests/test-v2v-i-ova-gz.expected | 1 - tests/test-v2v-i-ova-snapshots.expected | 1 - tests/test-v2v-i-ova-snapshots.expected2 | 1 - tests/test-v2v-i-ova-subfolders.expected | 1 - tests/test-v2v-i-ova-subfolders.expected2 | 1 - tests/test-v2v-i-ova-tar.expected | 1 - tests/test-v2v-i-ova-tar.expected2 | 1 - tests/test-v2v-i-ova-two-disks.expected | 1 - tests/test-v2v-i-ova-two-disks.expected2 | 1 - tests/test-v2v-i-vmx-1.expected | 1 - tests/test-v2v-i-vmx-2.expected | 1 - tests/test-v2v-i-vmx-3.expected | 1 - tests/test-v2v-i-vmx-4.expected | 1 - tests/test-v2v-i-vmx-5.expected | 1 - tests/test-v2v-print-source.expected | 1 - 21 files changed, 46 deletions(-) diff --git a/input/input.ml b/input/input.ml index c7ae8c5d96ce..9fbfe1276989 100644 --- a/input/input.ml +++ b/input/input.ml @@ -272,7 +272,6 @@ and disk_source cmdline args = s_display = Some { s_display_type = Window; s_keymap = None; s_password = None; s_listen = LNoListen; s_port = None }; - s_video = None; s_sound = None; s_disks = s_disks; s_removables = []; @@ -596,7 +595,6 @@ and ova_source _ args = s_features = []; (* XXX *) s_firmware = firmware; s_display = None; (* XXX *) - s_video = None; s_sound = None; s_disks = s_disks; s_removables = removables; diff --git a/input/parse_domain_from_vmx.ml b/input/parse_domain_from_vmx.ml index 650ddbb7bac1..d797e94ccfc4 100644 --- a/input/parse_domain_from_vmx.ml +++ b/input/parse_domain_from_vmx.ml @@ -412,13 +412,6 @@ let parse_domain_from_vmx vmx_source = warning (f_"unknown firmware value '%s', assuming BIOS") fw; BIOS in - let video = - if Parse_vmx.namespace_present vmx ["svga"] then - (* We could also parse svga.vramSize. *) - Some "vmvga" - else - None in - let sound = match Parse_vmx.get_string vmx ["sound"; "virtualDev"] with | Some "sb16" -> Some { s_sound_model = SB16 } @@ -445,7 +438,6 @@ let parse_domain_from_vmx vmx_source = s_features = []; s_firmware = firmware; s_display = None; - s_video = video; s_sound = sound; s_disks = List.map fst disks; s_removables = removables; diff --git a/input/parse_libvirt_xml.ml b/input/parse_libvirt_xml.ml index 835649939d22..1e98ce1a8694 100644 --- a/input/parse_libvirt_xml.ml +++ b/input/parse_libvirt_xml.ml @@ -183,19 +183,6 @@ let parse_libvirt_xml ?conn xml = None ) in - (* Video adapter. *) - let video = - let obj = Xml.xpath_eval_expression xpathctx "/domain/devices/video" in - let nr_nodes = Xml.xpathobj_nr_nodes obj in - if nr_nodes < 1 then None - else ( - (* Ignore everything except the first <video> device. *) - let node = Xml.xpathobj_node obj 0 in - - Xml.xpathctx_set_current_context xpathctx node; - xpath_string "model/@type" - ) in - (* Sound card. *) let sound = let obj = Xml.xpath_eval_expression xpathctx "/domain/devices/sound" in @@ -512,7 +499,6 @@ let parse_libvirt_xml ?conn xml = s_features = features; s_firmware = firmware; s_display = display; - s_video = video; s_sound = sound; s_disks = s_disks; s_removables = removables; diff --git a/lib/types.ml b/lib/types.ml index 9629bab50d4e..894391baecaa 100644 --- a/lib/types.ml +++ b/lib/types.ml @@ -36,7 +36,6 @@ type source = { s_features : string list; s_firmware : source_firmware; s_display : source_display option; - s_video : string option; s_sound : source_sound option; s_disks : source_disk list; s_removables : source_removable list; @@ -112,7 +111,6 @@ hypervisor type: %s CPU features: %s firmware: %s display: %s - video: %s sound: %s disks: %s @@ -136,9 +134,6 @@ NICs: (match s.s_display with | None -> "" | Some display -> string_of_source_display display) - (match s.s_video with - | None -> "" - | Some video -> video) (match s.s_sound with | None -> "" | Some sound -> string_of_source_sound sound) diff --git a/lib/types.mli b/lib/types.mli index 65efbcc03fec..02913c0cb4a1 100644 --- a/lib/types.mli +++ b/lib/types.mli @@ -70,7 +70,6 @@ type source = { s_features : string list; (** Machine features. *) s_firmware : source_firmware; (** Firmware (BIOS or EFI). *) s_display : source_display option; (** Guest display. *) - s_video : string option; (** Video adapter. *) s_sound : source_sound option; (** Sound card. *) s_disks : source_disk list; (** Source disks. *) s_removables : source_removable list; (** CDROMs etc. *) diff --git a/tests/test-v2v-i-ova-formats.expected b/tests/test-v2v-i-ova-formats.expected index dcc20fd0095f..f98ac82dba77 100644 --- a/tests/test-v2v-i-ova-formats.expected +++ b/tests/test-v2v-i-ova-formats.expected @@ -11,7 +11,6 @@ hypervisor type: vmware CPU features: firmware: uefi display: - video: sound: disks: 0 [scsi] diff --git a/tests/test-v2v-i-ova-gz.expected b/tests/test-v2v-i-ova-gz.expected index 9ff6ea59ca70..3e7474591f0e 100644 --- a/tests/test-v2v-i-ova-gz.expected +++ b/tests/test-v2v-i-ova-gz.expected @@ -11,7 +11,6 @@ hypervisor type: vmware CPU features: firmware: bios display: - video: sound: disks: 0 [scsi] diff --git a/tests/test-v2v-i-ova-snapshots.expected b/tests/test-v2v-i-ova-snapshots.expected index dcc20fd0095f..f98ac82dba77 100644 --- a/tests/test-v2v-i-ova-snapshots.expected +++ b/tests/test-v2v-i-ova-snapshots.expected @@ -11,7 +11,6 @@ hypervisor type: vmware CPU features: firmware: uefi display: - video: sound: disks: 0 [scsi] diff --git a/tests/test-v2v-i-ova-snapshots.expected2 b/tests/test-v2v-i-ova-snapshots.expected2 index 58e98594046f..dd4525ff9c42 100644 --- a/tests/test-v2v-i-ova-snapshots.expected2 +++ b/tests/test-v2v-i-ova-snapshots.expected2 @@ -11,7 +11,6 @@ hypervisor type: vmware CPU features: firmware: uefi display: - video: sound: disks: json:{ "file": { "driver": "raw", "offset": x, "size": 12288, "file": { "driver": "file", "filename": "test-snapshots.ova" } } } (vmdk) [scsi] diff --git a/tests/test-v2v-i-ova-subfolders.expected b/tests/test-v2v-i-ova-subfolders.expected index dcc20fd0095f..f98ac82dba77 100644 --- a/tests/test-v2v-i-ova-subfolders.expected +++ b/tests/test-v2v-i-ova-subfolders.expected @@ -11,7 +11,6 @@ hypervisor type: vmware CPU features: firmware: uefi display: - video: sound: disks: 0 [scsi] diff --git a/tests/test-v2v-i-ova-subfolders.expected2 b/tests/test-v2v-i-ova-subfolders.expected2 index e0dd1937d6f6..7b6c544fcc04 100644 --- a/tests/test-v2v-i-ova-subfolders.expected2 +++ b/tests/test-v2v-i-ova-subfolders.expected2 @@ -11,7 +11,6 @@ hypervisor type: vmware CPU features: firmware: uefi display: - video: sound: disks: json:{ "file": { "driver": "raw", "offset": x, "size": 10240, "file": { "driver": "file", "filename": "test.ova" } } } (vmdk) [scsi] diff --git a/tests/test-v2v-i-ova-tar.expected b/tests/test-v2v-i-ova-tar.expected index dcc20fd0095f..f98ac82dba77 100644 --- a/tests/test-v2v-i-ova-tar.expected +++ b/tests/test-v2v-i-ova-tar.expected @@ -11,7 +11,6 @@ hypervisor type: vmware CPU features: firmware: uefi display: - video: sound: disks: 0 [scsi] diff --git a/tests/test-v2v-i-ova-tar.expected2 b/tests/test-v2v-i-ova-tar.expected2 index 1fa120b9970d..77bc60cf7d05 100644 --- a/tests/test-v2v-i-ova-tar.expected2 +++ b/tests/test-v2v-i-ova-tar.expected2 @@ -11,7 +11,6 @@ hypervisor type: vmware CPU features: firmware: uefi display: - video: sound: disks: json:{ "file": { "driver": "raw", "offset": x, "size": 10240, "file": { "driver": "file", "filename": "test-tar.ova" } } } (vmdk) [scsi] diff --git a/tests/test-v2v-i-ova-two-disks.expected b/tests/test-v2v-i-ova-two-disks.expected index bf03c3e865ad..93a97b16f6f4 100644 --- a/tests/test-v2v-i-ova-two-disks.expected +++ b/tests/test-v2v-i-ova-two-disks.expected @@ -11,7 +11,6 @@ hypervisor type: vmware CPU features: firmware: bios display: - video: sound: disks: 0 [scsi] diff --git a/tests/test-v2v-i-ova-two-disks.expected2 b/tests/test-v2v-i-ova-two-disks.expected2 index 92728ab6a782..7c716ccd87c7 100644 --- a/tests/test-v2v-i-ova-two-disks.expected2 +++ b/tests/test-v2v-i-ova-two-disks.expected2 @@ -11,7 +11,6 @@ hypervisor type: vmware CPU features: firmware: bios display: - video: sound: disks: json:{ "file": { "driver": "raw", "offset": x, "size": 10240, "file": { "driver": "file", "filename": "test.ova" } } } (vmdk) [scsi] diff --git a/tests/test-v2v-i-vmx-1.expected b/tests/test-v2v-i-vmx-1.expected index e614e760e2be..bca99c48c5cb 100644 --- a/tests/test-v2v-i-vmx-1.expected +++ b/tests/test-v2v-i-vmx-1.expected @@ -11,7 +11,6 @@ hypervisor type: vmware CPU features: firmware: bios display: - video: vmvga sound: disks: 0 [scsi] diff --git a/tests/test-v2v-i-vmx-2.expected b/tests/test-v2v-i-vmx-2.expected index 4ecc6b63db0e..e56777c8509d 100644 --- a/tests/test-v2v-i-vmx-2.expected +++ b/tests/test-v2v-i-vmx-2.expected @@ -11,7 +11,6 @@ hypervisor type: vmware CPU features: firmware: bios display: - video: vmvga sound: disks: 0 [scsi] diff --git a/tests/test-v2v-i-vmx-3.expected b/tests/test-v2v-i-vmx-3.expected index afa549da9945..47e4d25eb53a 100644 --- a/tests/test-v2v-i-vmx-3.expected +++ b/tests/test-v2v-i-vmx-3.expected @@ -11,7 +11,6 @@ hypervisor type: vmware CPU features: firmware: uefi display: - video: vmvga sound: disks: 0 [scsi] diff --git a/tests/test-v2v-i-vmx-4.expected b/tests/test-v2v-i-vmx-4.expected index 919db78a25ac..dd7312945b69 100644 --- a/tests/test-v2v-i-vmx-4.expected +++ b/tests/test-v2v-i-vmx-4.expected @@ -11,7 +11,6 @@ hypervisor type: vmware CPU features: firmware: bios display: - video: vmvga sound: disks: 0 [scsi] diff --git a/tests/test-v2v-i-vmx-5.expected b/tests/test-v2v-i-vmx-5.expected index 85a37ca00f74..cfddcda73b9a 100644 --- a/tests/test-v2v-i-vmx-5.expected +++ b/tests/test-v2v-i-vmx-5.expected @@ -11,7 +11,6 @@ hypervisor type: vmware CPU features: firmware: bios display: - video: sound: disks: 0 [scsi] diff --git a/tests/test-v2v-print-source.expected b/tests/test-v2v-print-source.expected index cfeade1c012d..73473f9df877 100644 --- a/tests/test-v2v-print-source.expected +++ b/tests/test-v2v-print-source.expected @@ -9,7 +9,6 @@ hypervisor type: kvm CPU features: acpi,apic,pae firmware: unknown display: - video: vga sound: disks: 0 [virtio-blk] -- 2.19.1.3.g30247aa5d201
participants (3)
-
Kashyap Chamarthy -
Laszlo Ersek -
Liran Rotenberg