[libvirt] [PATCH 8/9] Factor out the method to get the PCI address of a controller for a given disk

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

 



We need this multiple times later on.

Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@xxxxxxxxxxx>
Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
---
 src/qemu_driver.c |  155 +++++++++++++++++++++++++++++++++--------------------
 1 files changed, 97 insertions(+), 58 deletions(-)

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index f1c2a45..ddc46f6 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -5370,44 +5370,20 @@ qemudParseDriveAddReply(virDomainObjPtr vm,
     return 0;
 }
 
-static int qemudDomainAttachDiskDevice(virConnectPtr conn,
-				       virDomainObjPtr vm,
-				       virDomainDeviceDefPtr dev)
-{
-    int ret, i;
-    char *cmd, *reply;
-    char *safe_path;
-    /* NOTE: Later patch will use type of the controller instead
-       of the disk */
-    const char* type = virDomainDiskBusTypeToString(dev->data.disk->bus);
-    int tryOldSyntax = 0;
-    unsigned domain, bus, slot;
-    int bus_id, unit_id;
-    virBuffer buf = VIR_BUFFER_INITIALIZER;
-    char* bus_unit_string;
-    int controller_specified;
-
-    for (i = 0 ; i < vm->def->ndisks ; i++) {
-        if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) {
-            qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
-                           _("target %s already exists"), dev->data.disk->dst);
-            return -1;
-        }
-    }
-
-    if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) {
-        virReportOOMError(conn);
-        return -1;
-    }
-
-try_command:
-    safe_path = qemudEscapeMonitorArg(dev->data.disk->src);
-    if (!safe_path) {
-        virReportOOMError(conn);
-        return -1;
-    }
+enum {
+    CONTROLLER_FOUND      =  0,
+    NO_CONTROLLER_FOUND   = -1, /* None specified, and none found */
+    CONTROLLER_INEXISTENT = -2, /* Controller specified, but not found */
+};
+static virDomainControllerDefPtr 
+qemudGetDiskController(virDomainObjPtr vm, virDomainDeviceDefPtr dev,
+                       int* ret) {
+    int controller_specified = 0;
+    int domain, bus, slot;
+    virDomainControllerDefPtr conPtr = NULL;
+    int i;
+    *ret = CONTROLLER_FOUND;
 
-    controller_specified = 0;
     /* Partially specified PCI addresses (should not happen, anyway)
        don't count as specification */
     if (dev->data.disk->controller_id != NULL || 
@@ -5422,38 +5398,28 @@ try_command:
            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)
+            if (vm->def->controllers[i]->type == dev->data.disk->type)
+                conPtr = vm->def->controllers[i]; 
                 break;
         }
         
-        if (i == vm->def->ncontrollers) {
-            qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
-                          _("Cannot find appropriate controller for hot-add"));
-            return -1;
+        if (!conPtr) {
+            *ret = NO_CONTROLLER_FOUND;
         }
-
-        domain = vm->def->controllers[i]->pci_addr.domain;
-        bus    = vm->def->controllers[i]->pci_addr.bus;
-        slot   = vm->def->controllers[i]->pci_addr.slot;
     }
 
     if (controller_specified && dev->data.disk->controller_id) {
         for (i = 0 ; i < vm->def->ncontrollers ; i++) {
             if (STREQ(dev->data.disk->controller_id, 
                       vm->def->controllers[i]->id)) {
+                conPtr = vm->def->controllers[i]; 
                 break;
             }
         }
         
-        if (i == vm->def->ncontrollers) {
-            qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
-                             _("Controller does not exist"));
-            return -1;
+        if (!conPtr) {
+            *ret = CONTROLLER_INEXISTENT;
         }
-        
-        domain = vm->def->controllers[i]->pci_addr.domain;
-        bus    = vm->def->controllers[i]->pci_addr.bus;
-        slot   = vm->def->controllers[i]->pci_addr.slot;
     } else if (controller_specified) {
         domain = dev->data.disk->controller_pci_addr.domain;
         bus    = dev->data.disk->controller_pci_addr.bus;
@@ -5463,19 +5429,92 @@ try_command:
             if (domain ==  vm->def->controllers[i]->pci_addr.domain &&
                 bus    ==  vm->def->controllers[i]->pci_addr.bus &&
                 slot   ==  vm->def->controllers[i]->pci_addr.slot)
+                conPtr = vm->def->controllers[i];
                 break;
         }
         
-        if (i == vm->def->ncontrollers) {
+        if (!conPtr) {
+            *ret = CONTROLLER_INEXISTENT;
+        }
+    }
+
+    return conPtr;
+}
+
+static int qemudDomainAttachDiskDevice(virConnectPtr conn,
+				       virDomainObjPtr vm,
+				       virDomainDeviceDefPtr dev)
+{
+    int ret, i;
+    char *cmd, *reply;
+    char *safe_path;
+    const char* type = NULL;
+    int tryOldSyntax = 0;
+    int bus_id, unit_id;
+    int domain, bus, slot;
+    virDomainControllerDefPtr conPtr;
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+    char* bus_unit_string;
+
+    /* Both controller and disk can specify a type like SCSI. While
+       a virtual disk as such is not bound to a specific bus type,
+       the specification is here for historical reasons. When controller
+       and disk type specification disagree, the former takes precedence
+       and we print a warning message, but still add the disk. */
+    if (virDiskHasValidController(dev->data.disk)) {
+        conPtr = qemudGetDiskController(vm, dev, &ret);
+        if (conPtr && dev->data.disk->bus != conPtr->type) {
+            VIR_WARN0(_("Controller and disk bus type disagree, controller " \
+                        "takes precedence\n"));
+            type = virDomainDiskBusTypeToString(conPtr->type);
+        }
+    }
+    if (!type) {
+        type = virDomainDiskBusTypeToString(dev->data.disk->bus);
+    }
+
+    for (i = 0 ; i < vm->def->ndisks ; i++) {
+        if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) {
+            qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+                           _("target %s already exists"), dev->data.disk->dst);
+            return -1;
+        }
+    }
+
+    if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) {
+        virReportOOMError(conn);
+        return -1;
+    }
+
+try_command:
+    safe_path = qemudEscapeMonitorArg(dev->data.disk->src);
+    if (!safe_path) {
+        virReportOOMError(conn);
+        return -1;
+    }
+
+    conPtr = qemudGetDiskController(vm, dev, &ret);
+    if (!conPtr) {
+        switch (ret) {
+        case CONTROLLER_INEXISTENT:
             qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                              _("Controller does not exist"));
             return -1;
+            break;  /* A bit pointless */
+        case NO_CONTROLLER_FOUND:
+            qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+                          _("Cannot find appropriate controller for hot-add"));
+            return -1;
+            break;
         }
     }
 
-    /* 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. */
+    /* At this point, we have a valid controller, regardless if an explicit
+       controller has been specified or not. */
+    domain = conPtr->pci_addr.domain;
+    bus = conPtr->pci_addr.bus;
+    slot = conPtr->pci_addr.slot;
+
     if (dev->data.disk->bus_id != -1) {
         virBufferVSprintf(&buf, ",bus=%d", dev->data.disk->bus_id);
     }
-- 
1.6.4

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]