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

[Daniel Henrique Barboza]

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