[libvirt] [PATCH] qemu: maintain user alias for video type 'none'
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 Signed-off-by: Jonathon Jongsma <jjongsma@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; /* 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
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".
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@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@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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Sep 06, 2019 at 09:58:32AM +0200, Erik Skultety wrote:
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".
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.
'Resolves:' is just redundant noise - people only do it out of habit because Red Hat's own internal dev tools for RHEL require that for beurocratic reasons. In any case, the most important rule is that the commit message should fully describe the problem. ie it should be possible to understand the commit without opening the BZ link at all. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (3)
-
Daniel P. Berrangé -
Erik Skultety -
Jonathon Jongsma