[Libguestfs] [virt-v2v PATCH wave 1] lib/types: remove "source video: QXL" constructor

[Richard W.M. Jones]

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