[libvirt] [PATCH v1 1/1] qemu_hotplug.c: assert disk->info.alias unique before hotplug
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'
Reported-by: Srikanth Aithal <bssrikanth(a)in.ibm.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
---
src/qemu/qemu_hotplug.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index a1c3ca999b..7c770211ab 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -875,6 +875,27 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
}
+/**
+ * qemuDomainDeviceAliasIsUnique:
+ *
+ * Searches the existing domain disks to check if a given alias is
+ * unique. */
+static bool
+qemuDomainDeviceAliasIsUnique(virDomainDefPtr def, char *alias)
+{
+ int idx;
+
+ for (idx = (def->ndisks - 1); idx >= 0; idx--) {
+ if (def->disks[idx]->info.alias &&
+ (strcmp(def->disks[idx]->info.alias, alias) == 0)) {
+
+ return false;
+ }
+ }
+ return true;
+}
+
+
/**
* qemuDomainAttachDiskGeneric:
*
@@ -897,6 +918,13 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
goto error;
+ if (!qemuDomainDeviceAliasIsUnique(vm->def, disk->info.alias)) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("attached disk conflicts with existing "
+ "device id '%s'"), disk->info.alias);
+ goto error;
+ }
+
if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
goto error;
--
2.20.1