[libvirt] [PATCH] qemu: maintain user alias for video type 'none'

[Erik Skultety]

On Thu, Sep 05, 2019 at 11:17:38AM -0500, Jonathon Jongsma wrote:
> After parsing a video device with a model type of
> VIR_DOMAIN_VIDEO_TYPE_NONE, all device info is cleared (see
> virDomainDefPostParseVideo()) in order to avoid formatting any
> auto-generated values for the XML. Subsequently, however, an alias is
> generated for the video device (e.g. 'video0'), which results in an
> alias property being formatted in the XML output anyway. This creates
> confusion if the user has explicitly provided an alias for the video
> device since the alias will change.
>
> To avoid this, don't clear the user-defined alias for video devices of
> type "none".
>
> See https://bugzilla.redhat.com/show_bug.cgi?id=1720612

s/See//   - we had a discussion about using/not using "Resolves: <url>" across
the commit messages; right now, the custom is to mention just the plain URL.

Btw. I had the very same question you posted to the BZ so thanks for asking.
It is a bit unfortunate, that we add a default <video> when there's a
<graphics> element present, because it makes things a bit complicated resulting
in introduction of a "none" device type, but it is what it is and I don't think
ovirt has really an alternative :(.

>
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---
>  src/conf/domain_conf.c                         | 7 +++++--
>  tests/qemuxml2argvdata/video-none-device.xml   | 1 +
>  tests/qemuxml2xmloutdata/video-none-device.xml | 1 +
>  3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 6c429cd593..d11d1fb903 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -5635,11 +5635,14 @@ virDomainDefPostParseVideo(virDomainDefPtr def,
>          return 0;
>
>      if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_NONE) {
> +        char *alias;

I'd put a blank line here to separate the declaration from everything else.

Reviewed-by: Erik Skultety <eskultet at redhat.com>

>          /* we don't want to format any values we automatically fill in for
> -         * videos into the XML, so clear them
> -         */
> +         * videos into the XML, so clear them, but retain any user-assigned
> +         * alias */
> +        VIR_STEAL_PTR(alias, def->videos[0]->info.alias);
>          virDomainVideoDefClear(def->videos[0]);
>          def->videos[0]->type = VIR_DOMAIN_VIDEO_TYPE_NONE;
> +        VIR_STEAL_PTR(def->videos[0]->info.alias, alias);
>      } else {
>          virDomainDeviceDef device = {
>              .type = VIR_DOMAIN_DEVICE_VIDEO,
> diff --git a/tests/qemuxml2argvdata/video-none-device.xml b/tests/qemuxml2argvdata/video-none-device.xml
> index 4b591562b7..c1c60c1d9e 100644
> --- a/tests/qemuxml2argvdata/video-none-device.xml
> +++ b/tests/qemuxml2argvdata/video-none-device.xml
> @@ -31,6 +31,7 @@
>      <graphics type='vnc'/>
>      <video>
>        <model type='none'/>
> +      <alias name='ua-user-video-alias'/>
>      </video>
>      <memballoon model='virtio'>
>        <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> diff --git a/tests/qemuxml2xmloutdata/video-none-device.xml b/tests/qemuxml2xmloutdata/video-none-device.xml
> index 6e76b394fe..82615f5125 100644
> --- a/tests/qemuxml2xmloutdata/video-none-device.xml
> +++ b/tests/qemuxml2xmloutdata/video-none-device.xml
> @@ -34,6 +34,7 @@
>      </graphics>
>      <video>
>        <model type='none'/>
> +      <alias name='ua-user-video-alias'/>
>      </video>
>      <memballoon model='virtio'>
>        <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> --
> 2.21.0
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list