[libvirt] New libvirt-tck failure with libvirt 5.2.0
Daniel Henrique Barboza
danielhb413 at gmail.com
Wed Apr 10 19:46:02 UTC 2019
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 at gmail.com>
>>>>> 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.
More information about the libvir-list
mailing list