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

[Daniel Henrique Barboza <danielhb413@xxxxxxxxx>]

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@xxxxxxxxxx>
Signed-off-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
---
 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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list