[virt-v2v PATCH wave 1] lib/types: remove "source video: QXL" constructor
* Background: We intend to stop virt-v2v from outputting converted domains with a QXL display device [1]. The reason is that QXL is a complex device model with a large security attack surface, while at the same time, better device models exist with regard to either guest compatibility, or graphics acceleration. Namely, if compatibility takes priority, standard VGA is at least as widely supported by guests [2] [3] [4], with lesser security risks. And if graphics performance takes priority, then 2D acceleration is generally considered obsolete, and virtio-vga is where new 3D features are being developed [5]. [1] https://bugzilla.redhat.com/show_bug.cgi?id=1961107#c1 [2] https://bugzilla.redhat.com/show_bug.cgi?id=1961107#c2 [3] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#qxl-vga [4] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#VGA [5] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#virtio-vga For virt-v2v in particular, compatibility is what matters, so standard VGA will ultimately supplant QXL in newly converted domains. * About this patch: In order to get the ball rolling, remove the "Source_QXL" constructor of the "source_video" union type. "Source_QXL" was originally introduced in commit [6], and then put to use in commit [7], specifically for the sake of in-place conversions, so that the "input" QXL display device serve as a distinguished hint for the "output" domain (after conversion). With the modularization of virt-v2v in commit [8], in-place conversion has been (temporarily) removed. Therefore, "Source_QXL" is functionally dead code, at the time of writing. [6] 04af2bc3cd2c ("v2v: collect source network and video adapter types", 2016-03-24) [7] 3c21ad036296 ("v2v: in-place: request caps based on source config", 2016-03-24) [8] 255722cbf39a ("v2v: Modular virt-v2v", 2021-09-07) "Functionally dead" means that, mentions of the QXL display device in the *source* domain descriptions of the test suite can be satisfied by the parametric "Source_other_video of string" constructor just as well; there is no actual functionality attached to "Source_QXL" at the moment. And once we reimplement in-place conversion for modular virt-v2v, we'll still want to flip domains that use QXL to standard VGA (see the background above). (Further patches are expected for RHBZ#1961107.) Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- lib/types.ml | 4 +--- lib/types.mli | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/types.ml b/lib/types.ml index 27f488255fd7..72d10d1ec774 100644 --- a/lib/types.ml +++ b/lib/types.ml @@ -90,7 +90,7 @@ and s_display_listen = | LNone and source_video = Source_other_video of string | - Source_Cirrus | Source_QXL + Source_Cirrus and source_sound = { s_sound_model : source_sound_model; @@ -260,12 +260,10 @@ and string_of_source_display { s_display_type = typ; ) and string_of_source_video = function - | Source_QXL -> "qxl" | Source_Cirrus -> "cirrus" | Source_other_video video -> video and source_video_of_string = function - | "qxl" -> Source_QXL | "cirrus" -> Source_Cirrus | video -> Source_other_video video diff --git a/lib/types.mli b/lib/types.mli index 2af5871fd54b..0bae445d8393 100644 --- a/lib/types.mli +++ b/lib/types.mli @@ -147,7 +147,7 @@ and s_display_listen = (** Video adapter model. *) and source_video = Source_other_video of string | - Source_Cirrus | Source_QXL + Source_Cirrus and source_sound = { s_sound_model : source_sound_model; (** Sound model. *) base-commit: 7ce5df273be304ec3ba87e46319cf78275eb4990 -- 2.19.1.3.g30247aa5d201
On Wed, Oct 20, 2021 at 06:35:03PM +0200, Laszlo Ersek wrote:
* Background:
We intend to stop virt-v2v from outputting converted domains with a QXL display device [1]. The reason is that QXL is a complex device model with a large security attack surface, while at the same time, better device models exist with regard to either guest compatibility, or graphics acceleration. Namely, if compatibility takes priority, standard VGA is at least as widely supported by guests [2] [3] [4], with lesser security risks. And if graphics performance takes priority, then 2D acceleration is generally considered obsolete, and virtio-vga is where new 3D features are being developed [5].
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1961107#c1 [2] https://bugzilla.redhat.com/show_bug.cgi?id=1961107#c2 [3] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#qxl-vga [4] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#VGA [5] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#virtio-vga
For virt-v2v in particular, compatibility is what matters, so standard VGA will ultimately supplant QXL in newly converted domains.
* About this patch:
In order to get the ball rolling, remove the "Source_QXL" constructor of the "source_video" union type.
"Source_QXL" was originally introduced in commit [6], and then put to use in commit [7], specifically for the sake of in-place conversions, so that the "input" QXL display device serve as a distinguished hint for the "output" domain (after conversion).
With the modularization of virt-v2v in commit [8], in-place conversion has been (temporarily) removed. Therefore, "Source_QXL" is functionally dead code, at the time of writing.
[6] 04af2bc3cd2c ("v2v: collect source network and video adapter types", 2016-03-24) [7] 3c21ad036296 ("v2v: in-place: request caps based on source config", 2016-03-24) [8] 255722cbf39a ("v2v: Modular virt-v2v", 2021-09-07)
"Functionally dead" means that, mentions of the QXL display device in the *source* domain descriptions of the test suite can be satisfied by the parametric "Source_other_video of string" constructor just as well; there is no actual functionality attached to "Source_QXL" at the moment. And once we reimplement in-place conversion for modular virt-v2v, we'll still want to flip domains that use QXL to standard VGA (see the background above).
(Further patches are expected for RHBZ#1961107.)
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- lib/types.ml | 4 +--- lib/types.mli | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/lib/types.ml b/lib/types.ml index 27f488255fd7..72d10d1ec774 100644 --- a/lib/types.ml +++ b/lib/types.ml @@ -90,7 +90,7 @@ and s_display_listen = | LNone
and source_video = Source_other_video of string | - Source_Cirrus | Source_QXL + Source_Cirrus
and source_sound = { s_sound_model : source_sound_model; @@ -260,12 +260,10 @@ and string_of_source_display { s_display_type = typ; )
and string_of_source_video = function - | Source_QXL -> "qxl" | Source_Cirrus -> "cirrus" | Source_other_video video -> video
and source_video_of_string = function - | "qxl" -> Source_QXL | "cirrus" -> Source_Cirrus | video -> Source_other_video video
diff --git a/lib/types.mli b/lib/types.mli index 2af5871fd54b..0bae445d8393 100644 --- a/lib/types.mli +++ b/lib/types.mli @@ -147,7 +147,7 @@ and s_display_listen =
(** Video adapter model. *) and source_video = Source_other_video of string | - Source_Cirrus | Source_QXL + Source_Cirrus
and source_sound = { s_sound_model : source_sound_model; (** Sound model. *)
base-commit: 7ce5df273be304ec3ba87e46319cf78275eb4990 --
A neutral change that doesn't affect current virt-v2v at all, and won't affect in-place conversions in future because we won't want to install the QXL driver for the reasons outlined in the commit message. Therefore: ACK Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
On 10/20/21 18:46, Richard W.M. Jones wrote:
On Wed, Oct 20, 2021 at 06:35:03PM +0200, Laszlo Ersek wrote:
* Background:
We intend to stop virt-v2v from outputting converted domains with a QXL display device [1]. The reason is that QXL is a complex device model with a large security attack surface, while at the same time, better device models exist with regard to either guest compatibility, or graphics acceleration. Namely, if compatibility takes priority, standard VGA is at least as widely supported by guests [2] [3] [4], with lesser security risks. And if graphics performance takes priority, then 2D acceleration is generally considered obsolete, and virtio-vga is where new 3D features are being developed [5].
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1961107#c1 [2] https://bugzilla.redhat.com/show_bug.cgi?id=1961107#c2 [3] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#qxl-vga [4] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#VGA [5] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#virtio-vga
For virt-v2v in particular, compatibility is what matters, so standard VGA will ultimately supplant QXL in newly converted domains.
* About this patch:
In order to get the ball rolling, remove the "Source_QXL" constructor of the "source_video" union type.
"Source_QXL" was originally introduced in commit [6], and then put to use in commit [7], specifically for the sake of in-place conversions, so that the "input" QXL display device serve as a distinguished hint for the "output" domain (after conversion).
With the modularization of virt-v2v in commit [8], in-place conversion has been (temporarily) removed. Therefore, "Source_QXL" is functionally dead code, at the time of writing.
[6] 04af2bc3cd2c ("v2v: collect source network and video adapter types", 2016-03-24) [7] 3c21ad036296 ("v2v: in-place: request caps based on source config", 2016-03-24) [8] 255722cbf39a ("v2v: Modular virt-v2v", 2021-09-07)
"Functionally dead" means that, mentions of the QXL display device in the *source* domain descriptions of the test suite can be satisfied by the parametric "Source_other_video of string" constructor just as well; there is no actual functionality attached to "Source_QXL" at the moment. And once we reimplement in-place conversion for modular virt-v2v, we'll still want to flip domains that use QXL to standard VGA (see the background above).
(Further patches are expected for RHBZ#1961107.)
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- lib/types.ml | 4 +--- lib/types.mli | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/lib/types.ml b/lib/types.ml index 27f488255fd7..72d10d1ec774 100644 --- a/lib/types.ml +++ b/lib/types.ml @@ -90,7 +90,7 @@ and s_display_listen = | LNone
and source_video = Source_other_video of string | - Source_Cirrus | Source_QXL + Source_Cirrus
and source_sound = { s_sound_model : source_sound_model; @@ -260,12 +260,10 @@ and string_of_source_display { s_display_type = typ; )
and string_of_source_video = function - | Source_QXL -> "qxl" | Source_Cirrus -> "cirrus" | Source_other_video video -> video
and source_video_of_string = function - | "qxl" -> Source_QXL | "cirrus" -> Source_Cirrus | video -> Source_other_video video
diff --git a/lib/types.mli b/lib/types.mli index 2af5871fd54b..0bae445d8393 100644 --- a/lib/types.mli +++ b/lib/types.mli @@ -147,7 +147,7 @@ and s_display_listen =
(** Video adapter model. *) and source_video = Source_other_video of string | - Source_Cirrus | Source_QXL + Source_Cirrus
and source_sound = { s_sound_model : source_sound_model; (** Sound model. *)
base-commit: 7ce5df273be304ec3ba87e46319cf78275eb4990 --
A neutral change that doesn't affect current virt-v2v at all, and won't affect in-place conversions in future because we won't want to install the QXL driver for the reasons outlined in the commit message. Therefore:
ACK
Commit 17e7cbffb9e5. Thanks! Laszlo
participants (2)
-
Laszlo Ersek -
Richard W.M. Jones