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@xxxxxxxxxx> > --- > 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@xxxxxxxxxx> > /* 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@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
