[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v1 1/1] qemu_hotplug.c: assert disk->info.alias unique before hotplug





On 1/18/19 5:13 AM, Peter Krempa wrote:
On Thu, Jan 17, 2019 at 17:23:07 -0200, Daniel Henrique Barboza wrote:
In a case where we want to hotplug the following disk:

<disk type='file' device='disk'>
     (...)
     <address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>

In a QEMU guest that has a single OS disk, as follows:

<disk type='file' device='disk'>
     (...)
     <address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>

What happens is that the existing guest disk will receive the ID
'scsi0-0-0-0' due to how Libvirt calculate the alias based on
the address in qemu_alias.c, qemuAssignDeviceDiskAlias. When hotplugging
a disk that happens to have the same address, Libvirt will calculate
the same ID to it and attempt to device_add. QEMU will refuse it:

$ virsh attach-device dhb hp-disk-dup.xml
error: Failed to attach device from hp-disk-dup.xml
error: internal error: unable to execute QEMU command 'device_add': Duplicate ID 'scsi0-0-0-0' for device

And Libvirt follows it up with a cleanup code in qemuDomainAttachDiskGeneric
that ends up removing what supposedly is a faulty hotplugged disk but, in
this case, ends up being the original guest disk. This happens because Libvirt
doesn't differentiate the error received by QMP device_add.

An argument can be made for how QMP device_add should provide a different
error code for this scenario. A quicker way to solve the problem is
presented in this patch: let us check the generated alias against the
aliases already presented in the disks in the VM definition. If a match
happens, error out without calling device_add.

After this patch, this is the result of the previous attach-device call:

$ ./run tools/virsh attach-device dhb ~/hp-disk-dup.xml
[sudo] password for danielhb:
error: Failed to attach device from /home/danielhb/hp-disk-dup.xml
error: operation failed: attached disk conflicts with existing device id 'scsi0-0-0-0'
The main problem here is that we don't check the duplicity of the
<address> data not the alias itself. The alias is only the first symptom
of this which might not occur in some configurations e.g. if the other
disk with the same alias uses an useralias.

I'd prefer if this is fixed properly by fixing/adding the address
check.

Thanks for the review. I'll respin it with an address check instead of the alias
check done here.




DHB


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]