[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 09/13] Remove surprises in the semantics of disk-hotadd



From: Wolfgang Mauerer <wolfgang mauerer siemens com>

When a disk is added without an explicitly specified
controller as host, then try to find the first available
controller. If none exists, do not (as in previous versions)
add a new PCI controller device with the disk attached,
but bail out with an error. Notice that this patch changes
the behaviour as compared to older libvirt releases, as
has been discussed on the mailing list (see
http://thread.gmane.org/gmane.comp.emulators.libvirt/15860)

Signed-off-by: Wolfgang Mauerer <wolfgang mauerer siemens com>
Signed-off-by: Jan Kiszka <jan kiszka siemens com>
---
 src/qemu/qemu_driver.c |  122 +++++++++++++++++++++++++++---------------------
 1 files changed, 68 insertions(+), 54 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d3395a7..4f647c4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4406,68 +4406,82 @@ try_command:
         controller_specified = 1;
     }
 
-    if (controller_specified) {
-        if (dev->data.disk->controller_name) {
-            for (i = 0 ; i < vm->def->ncontrollers ; i++) {
-                if (STREQ(dev->data.disk->controller_name,
-                          vm->def->controllers[i]->name)) {
-                    break;
-                }
-            }
-
-            if (i == vm->def->ncontrollers) {
-                qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
-                                 _("Controller does not exist"));
-                return -1;
-            }
-
-            domain = vm->def->controllers[i]->address->data.pci.domain;
-            bus    = vm->def->controllers[i]->address->data.pci.bus;
-            slot   = vm->def->controllers[i]->address->data.pci.slot;
-        } else {
-            domain = dev->data.disk->controller_pci_addr.domain;
-            bus    = dev->data.disk->controller_pci_addr.bus;
-            slot   = dev->data.disk->controller_pci_addr.slot;
-
-            for (i = 0 ; i < vm->def->ncontrollers ; i++) {
-                if (domain == vm->def->controllers[i]->address->data.pci.domain &&
-                    bus    == vm->def->controllers[i]->address->data.pci.bus &&
-                    slot   == vm->def->controllers[i]->address->data.pci.slot)
-                    break;
-            }
-
-            if (i == vm->def->ncontrollers) {
-                qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
-                                 _("Controller does not exist"));
-                return -1;
+   if (!controller_specified) {
+       /* Find an appropriate controller for disk-hotadd. Notice that
+          the admissible controller types (SCSI, virtio) have already
+          been checked in qemudDomainAttachDevice. */
+       for (i = 0 ; i < vm->def->ncontrollers ; i++) {
+           if (vm->def->controllers[i]->type == dev->data.disk->type)
+               break;
+       }
+
+       if (i == vm->def->ncontrollers) {
+           qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+                         _("Cannot find appropriate controller for hot-add"));
+           return -1;
+       }
+
+       domain = vm->def->controllers[i]->address->data.pci.domain;
+       bus    = vm->def->controllers[i]->address->data.pci.bus;
+       slot   = vm->def->controllers[i]->address->data.pci.slot;
+   }
+
+   if (controller_specified && dev->data.disk->controller_name) {
+       for (i = 0 ; i < vm->def->ncontrollers ; i++) {
+           if (STREQ(dev->data.disk->controller_name,
+                     vm->def->controllers[i]->name)) {
+               break;
             }
-	}
-
-        if (dev->data.disk->bus_id != -1) {
-            virBufferVSprintf(&buf, ",bus=%d", dev->data.disk->bus_id);
         }
 
-        if (dev->data.disk->unit_id != -1) {
-            virBufferVSprintf(&buf, ",unit=%d", dev->data.disk->unit_id);
+       if (i == vm->def->ncontrollers) {
+           qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+                            _("Controller does not exist"));
+           return -1;
+       }
+
+       domain = vm->def->controllers[i]->address->data.pci.domain;
+       bus    = vm->def->controllers[i]->address->data.pci.bus;
+       slot   = vm->def->controllers[i]->address->data.pci.slot;
+   } else if (controller_specified) {
+       domain = dev->data.disk->controller_pci_addr.domain;
+       bus    = dev->data.disk->controller_pci_addr.bus;
+       slot   = dev->data.disk->controller_pci_addr.slot;
+
+       for (i = 0 ; i < vm->def->ncontrollers ; i++) {
+           if (domain ==  vm->def->controllers[i]->address->data.pci.domain &&
+               bus    ==  vm->def->controllers[i]->address->data.pci.bus &&
+               slot   ==  vm->def->controllers[i]->address->data.pci.slot)
+               break;
         }
 
-        bus_unit_string = virBufferContentAndReset(&buf);
-
-        VIR_DEBUG ("%s: drive_add %d:%d:%d file=%s,if=%s%s", vm->def->name,
-                   domain, bus, slot, safe_path, type, bus_unit_string);
-        ret = virAsprintf(&cmd, "drive_add %s%d:%d:%d file=%s,if=%s%s",
-			  (tryOldSyntax ? "" : "pci_addr="), domain, bus,
-                          slot, safe_path, type, bus_unit_string);
+       if (i == vm->def->ncontrollers) {
+           qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+                            _("Controller does not exist"));
+           return -1;
+       }
+   }
+   /* At this point, we have a valid (domain, bus, slot) triple
+      that identifies the controller, regardless if an explicit
+      controller has been given or not. */
+   if (dev->data.disk->bus_id != -1) {
+       virBufferVSprintf(&buf, ",bus=%d", dev->data.disk->bus_id);
     }
-    else {
-        /* Old-style behaviour: If no <controller> reference is given, then
-           hotplug a new controller with the disk attached. */
-        VIR_DEBUG ("%s: pci_add %s storage file=%s,if=%s", vm->def->name,
-                   (tryOldSyntax ? "0": "pci_addr=auto"), safe_path, type);
-        ret = virAsprintf(&cmd, "pci_add %s storage file=%s,if=%s",
-                   (tryOldSyntax ? "0": "pci_addr=auto"), safe_path, type);
+
+   if (dev->data.disk->unit_id != -1) {
+       virBufferVSprintf(&buf, ",unit=%d", dev->data.disk->unit_id);
     }
 
+   bus_unit_string = virBufferContentAndReset(&buf);
+
+   VIR_DEBUG ("%s: drive_add %d:%d:%d file=%s,if=%s%s", vm->def->name,
+              domain, bus, slot, safe_path, type,
+              bus_unit_string ? bus_unit_string : "");
+   ret = virAsprintf(&cmd, "drive_add %s%d:%d:%d file=%s,if=%s%s",
+                     (tryOldSyntax ? "" : "pci_addr="), domain, bus,
+                     slot, safe_path, type,
+                     bus_unit_string ? bus_unit_string : "");
+
     VIR_FREE(safe_path);
     if (ret == -1) {
         virReportOOMError(conn);
-- 
1.6.4


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]