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(a)redhat.com&gt; > --- > 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