[Qemu-devel] [PATCH 9/9] Implement disk- and controller hotremove

[Wolfgang Mauerer]

Since both disks and disk controllers can be dynamically
added to the system, it makes sense to be also able to remove
them.

Signed-off-by: Wolfgang Mauerer <address@hidden>
Signed-off-by: Jan Kiszka <address@hidden>
---
 src/domain_conf.h |    8 +++
 src/qemu_driver.c |  161 +++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 153 insertions(+), 16 deletions(-)

diff --git a/src/domain_conf.h b/src/domain_conf.h
index 17f8b14..c7d49cf 100644
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -149,6 +149,14 @@ virDiskHasValidPciAddr(virDomainDiskDefPtr def)
     return def->pci_addr.domain || def->pci_addr.domain || def->pci_addr.slot;
 }
 
+static inline int
+virDiskHasValidController(virDomainDiskDefPtr def)
+{
+    return def->controller_id != NULL ||
+        def->controller_pci_addr.domain || def->controller_pci_addr.domain 
+        || def->controller_pci_addr.slot;
+}
+
 
 /* Two types of disk backends */
 enum virDomainFSType {
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index ddc46f6..5ac89a1 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -6204,32 +6204,31 @@ cleanup:
     return ret;
 }
 
-static int qemudDomainDetachPciDiskDevice(virConnectPtr conn,
-                                          virDomainObjPtr vm, 
virDomainDeviceDefPtr dev)
+static int qemudDomainDetachDiskController(virConnectPtr conn,
+                                           virDomainObjPtr vm, 
virDomainDeviceDefPtr dev)
 {
     int i, ret = -1;
     char *cmd = NULL;
     char *reply = NULL;
-    virDomainDiskDefPtr detach = NULL;
+    virDomainControllerDefPtr detach = NULL;
     int tryOldSyntax = 0;
 
-    for (i = 0 ; i < vm->def->ndisks ; i++) {
-        if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) {
-            detach = vm->def->disks[i];
+//    virDomainControllerDefPtr is dev->data.controller
+    for (i = 0 ; i < vm->def->ncontrollers ; i++) {
+        if (vm->def->controllers[i]->pci_addr.domain == 
+            dev->data.controller->pci_addr.domain &&
+            vm->def->controllers[i]->pci_addr.bus == 
+            dev->data.controller->pci_addr.bus &&
+            vm->def->controllers[i]->pci_addr.slot == 
+            dev->data.controller->pci_addr.slot) {
+            detach = vm->def->controllers[i];
             break;
         }
     }
 
     if (!detach) {
         qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
-                         _("disk %s not found"), dev->data.disk->dst);
-        goto cleanup;
-    }
-
-    if (!virDiskHasValidPciAddr(detach)) {
-        qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
-                         _("disk %s cannot be detached - no PCI address for 
device"),
-                           detach->dst);
+                         _("Controller %s not found"), dev->data.disk->dst);
         goto cleanup;
     }
 
@@ -6251,7 +6250,9 @@ try_command:
 
     if (qemudMonitorCommand(vm, cmd, &reply) < 0) {
         qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
-                          _("failed to execute detach disk %s command"), 
detach->dst);
+                         _("failed to execute detach controller %d:%d:%d " \
+                           "command"), detach->pci_addr.domain,
+                         detach->pci_addr.bus, detach->pci_addr.slot);
         goto cleanup;
     }
 
@@ -6267,6 +6268,124 @@ try_command:
     if (strstr(reply, "invalid slot") ||
         strstr(reply, "Invalid pci address")) {
         qemudReportError (conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+                          _("failed to detach controller: invalid PCI address 
%.4x:%.2x:%.2x: %s"),
+                          detach->pci_addr.domain,
+                          detach->pci_addr.bus,
+                          detach->pci_addr.slot,
+                          reply);
+        goto cleanup;
+    }
+
+    if (vm->def->ncontrollers > 1) {
+        vm->def->controllers[i] = 
vm->def->controllers[--vm->def->ncontrollers];
+        if (VIR_REALLOC_N(vm->def->controllers, vm->def->ncontrollers) < 0) {
+            virReportOOMError(conn);
+            goto cleanup;
+        }
+        qsort(vm->def->disks, vm->def->ncontrollers, 
+              sizeof(*vm->def->controllers),
+              virDomainControllerQSort);
+    } else {
+        VIR_FREE(vm->def->controllers[0]);
+        vm->def->controllers = 0;
+    }
+    ret = 0;
+
+cleanup:
+    VIR_FREE(reply);
+    VIR_FREE(cmd);
+    return ret;
+}
+
+static int qemudDomainDetachDiskDevice(virConnectPtr conn,
+                                       virDomainObjPtr vm, 
virDomainDeviceDefPtr dev)
+{
+    int i, ret = -1;
+    char *cmd = NULL;
+    char *reply = NULL;
+    const char* type;
+    virDomainDiskDefPtr detach = NULL;
+    int tryOldSyntax = 0;
+
+    /* If bus and unit are specified, use these first to identify
+       the disk */
+    if (dev->data.disk->controller_pci_addr.domain != -1) {
+        for (i = 0 ; i < vm->def->ndisks ; i++) {
+            if (dev->data.disk->bus_id != -1 &&
+                dev->data.disk->bus_id == vm->def->disks[i]->bus_id &&
+                dev->data.disk->unit_id != -1 &&
+                dev->data.disk->unit_id == vm->def->disks[i]->unit_id) {
+                detach = vm->def->disks[i];
+                break;
+            }
+        }
+    }
+
+    /* If the device did not specify a controller explicitely (and therefore
+       lacks a bus and unit specification), revert to the explicit target
+       device specification to identify a device for removal. */
+    if (!detach) {
+        for (i = 0 ; i < vm->def->ndisks ; i++) {
+            if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) {
+                detach = vm->def->disks[i];
+                break;
+            }
+        }
+    }
+
+    if (!detach) {
+        qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+                         _("disk %s not found"), dev->data.disk->dst);
+        goto cleanup;
+    }
+
+    if (!virDiskHasValidPciAddr(detach) && !virDiskHasValidController(detach)) 
{
+        qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+                         _("disk %s cannot be detached - no PCI address for 
device"),
+                           detach->dst);
+        goto cleanup;
+    }
+
+    type = virDomainDiskBusTypeToString(detach->bus);
+
+try_command:
+    if (tryOldSyntax) {
+        if (virAsprintf(&cmd, "drive_del 0 %d:%d:%d bus=%d,unit=%d,if=%s", 
+                        detach->pci_addr.domain, detach->pci_addr.bus, 
+                        detach->pci_addr.slot, detach->bus_id, 
+                        detach->unit_id, type) < 0) {
+            virReportOOMError(conn);
+            goto cleanup;
+        }
+    } else {
+        if (virAsprintf(&cmd, "drive_del pci_addr=%d:%d:%d 
bus=%d,unit=%d,if=%s",
+                        detach->pci_addr.domain,
+                        detach->pci_addr.bus,
+                        detach->pci_addr.slot, detach->bus_id, 
+                        detach->unit_id, type) < 0) {
+            virReportOOMError(conn);
+            goto cleanup;
+        }
+    }
+
+    if (qemudMonitorCommand(vm, cmd, &reply) < 0) {
+        qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+                          _("failed to execute detach disk %s command"), 
detach->dst);
+        goto cleanup;
+    }
+
+    DEBUG ("%s: drive_del reply: %s",vm->def->name,  reply);
+
+    if (!tryOldSyntax &&
+        strstr(reply, "extraneous characters")) {
+        tryOldSyntax = 1;
+        goto try_command;
+    }
+    /* OK bux x, unit x is printed on success, but this does not provide
+       any new information to us.*/
+    if (strstr(reply, "Invalid pci address") ||
+        strstr(reply, "No pci device with address")) {
+        qemudReportError (conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
                           _("failed to detach disk %s: invalid PCI address 
%.4x:%.2x:%.2x: %s"),
                           detach->dst,
                           detach->pci_addr.domain,
@@ -6274,6 +6393,11 @@ try_command:
                           detach->pci_addr.slot,
                           reply);
         goto cleanup;
+    } else if (strstr(reply, "Need to specify bus") ||
+               strstr(reply, "Need to specify unit")) {
+        qemudReportError (conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+                          _("failed to detach disk %s: bus and unit not 
(internal error)"),
+                          detach->dst);
     }
 
     if (vm->def->ndisks > 1) {
@@ -6575,7 +6699,7 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
         dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK &&
         (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI ||
          dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO)) {
-        ret = qemudDomainDetachPciDiskDevice(dom->conn, vm, dev);
+        ret = qemudDomainDetachDiskDevice(dom->conn, vm, dev);
         if (driver->securityDriver)
             driver->securityDriver->domainRestoreSecurityImageLabel(dom->conn, 
dev->data.disk);
         if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 1) < 0)
@@ -6584,6 +6708,11 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
         ret = qemudDomainDetachNetDevice(dom->conn, vm, dev);
     } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
         ret = qemudDomainDetachHostDevice(dom->conn, driver, vm, dev);
+    } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) {
+        /* NOTE: We can unplug the controller without having to
+           check if all disks are unplugged; it is at the user's 
+           responsibility to use the required OS services first. */
+        ret = qemudDomainDetachDiskController(dom->conn, vm, dev);
     } else
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
                          "%s", _("only SCSI or virtio disk device can be 
detached dynamically"));
-- 
1.6.4