domain_conf: check device address before attach

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 ub1810 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 patch adds an address verification for all attached devices, avoid
calling the driver attach() function using a device with duplicated address.
The change is done in virDomainDefCompatibleDevice when @action is equal
to VIR_DOMAIN_DEVICE_ACTION_ATTACH. The affected callers are:

- qemuDomainAttachDeviceLiveAndConfig, both LIVE and CONFIG cases;
- lxcDomainAttachDeviceFlags, both LIVE and CONFIG.

The check is done using the virDomainDefHasDeviceAddress, a generic
function that can check address duplicates for all supported device
types, not limiting just to DeviceDisk type.

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

$ ./run tools/virsh attach-device ub1810 hp-disk-dup.xml
error: Failed to attach device from hp-disk-dup.xml
error: Requested operation is not valid: Domain already contains a device with the same address

Reported-by: Srikanth Aithal <bssrikanth@in.ibm.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This commit is contained in:
Daniel Henrique Barboza 2019-03-15 18:06:45 -03:00 committed by Michal Privoznik
parent 530c1671e1
commit f1d6585300

View File

@ -28589,6 +28589,15 @@ virDomainDefCompatibleDevice(virDomainDefPtr def,
if (oldDev)
data.oldInfo = virDomainDeviceGetInfo(oldDev);
if (action == VIR_DOMAIN_DEVICE_ACTION_ATTACH &&
data.newInfo &&
data.newInfo->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
virDomainDefHasDeviceAddress(def, data.newInfo)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("Domain already contains a device with the same address"));
return -1;
}
if (action == VIR_DOMAIN_DEVICE_ACTION_UPDATE &&
live &&
(data.newInfo && data.oldInfo &&