On 4/10/19 4:33 PM, Jim Fehlig wrote:
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@xxxxxxxxx>
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.
This can be a good discussion for a mid-long term change. Changing an API
behavior without proper warning and communication tends to generate a lot of
bugs and angry users :)
For now, let's leave the tck test alone and restore this use of
attach-device.
DHB
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.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
