Laszlo Ersek
2021-Oct-20 16:35 UTC
[Libguestfs] [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 at 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
Richard W.M. Jones
2021-Oct-20 16:46 UTC
[Libguestfs] [virt-v2v PATCH wave 1] lib/types: remove "source video: QXL" constructor
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 at 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