Re: [libvirt] New libvirt-tck failure with libvirt 5.2.0

On 4/10/19 1:17 PM, Daniel Henrique Barboza wrote: > > > On 4/10/19 3:33 PM, Jim Fehlig wrote: >> On 4/10/19 12:25 PM, Daniel Henrique Barboza wrote: >>> >>> >>> On 4/10/19 3:08 PM, Jim Fehlig wrote: >>>> I noticed libvirt-tck test domain/207-disk-media-change.t started >>>> failing after updating to libvirt 5.2.0. A bisection fingered >>>> commit f1d65853 >>>> >>>> commit f1d6585300001c7b23b8796a0faa4411c3531996 >>>> Author: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; >>>> Date:   Fri Mar 15 18:06:45 2019 -0300 >>>> >>>>     domain_conf: check device address before attach >>>> >>>> This commit prevents a media change on CDROM devices. >>> >>> Thanks for letting us know. I'll run libvirt-tck (any pointers are >>> appreciated - >>> first time I've heard about this test suite) and see if I can get a >>> fix going. >> >> It's easy to reproduce outside of libvirt-tck. Simply start a VM >> with a cdrom device and then try to update the <source> element with >> 'virsh attach-device ...'. E.g. with the VM running try to "eject" >> the cdrom >> >> # cat update-empty-cdrom.xml >> <disk type='file' device='cdrom'> >>   <target dev='hdc'/> >> </disk> >> >> # virsh attach-device test update-empty-cdrom.xml >> error: Failed to attach device from update-empty-cdrom.xml >> error: Requested operation is not valid: Domain already contains a >> device with the same address > > > To give a background on this patch, this was intended to avoid a > situation > where a hotplug of a device with the same ID would fall back to a > cleanup > code that ended up unplugging the previous existing device in the guest. > This was happening when using the QEMU driver. Understood. And fixing that bug is a good thing! > What I didn't expect is for a CD-ROM media to be changed using a > attach-device command. I think that the CD-ROM type has specific > rules of not > hot-unplugging itself (i.e. the driver) in the situation I described > above. What > I was expecting for CDROM change media was a > VIR_DOMAIN_DEVICE_ACTION_UPDATE > action instead, so this usage completely went under the radar for that > patch. I don't particularly like overloading the meaning of attach-device in this way, but prohibiting it may be an unexpected change in behavior for some user. However if folks agree with the change we can simply adjust the test.

Regards, Jim > In fact, there are "update-device" and "change-media" commands that, > according to the man page, are meant for the purpose of media change. > However, the same man page doesn't prohibit the use of attach-device > for media change either. I am not sure why have a change-media command > if attach-device is supposed to handle media change as well, but for > now let's fix the usage of attach-device with CDROM.