[libvirt] [PATCH 7/9] Remove surprises in the semantics of disk-hotadd

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

 



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@xxxxxxxxxxx>
Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
---
 src/qemu_driver.c |  172 ++++++++++++++++++++++++++---------------------------
 1 files changed, 85 insertions(+), 87 deletions(-)

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 990f05a..f1c2a45 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -5417,68 +5417,81 @@ try_command:
         controller_specified = 1;
     }
 
-    if (controller_specified) {
-        if (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)) {
-                    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]->pci_addr.domain;
-            bus    = vm->def->controllers[i]->pci_addr.bus;
-            slot   = vm->def->controllers[i]->pci_addr.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]->pci_addr.domain &&
-                    bus    ==  vm->def->controllers[i]->pci_addr.bus &&
-                    slot   ==  vm->def->controllers[i]->pci_addr.slot)
-                    break;
-            }
+        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 (i == vm->def->ncontrollers) {
-                qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
-                                 _("Controller does not exist"));
-                return -1;
+    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)) {
+                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]->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;
+        slot   = dev->data.disk->controller_pci_addr.slot;
+        
+        for (i = 0 ; i < vm->def->ncontrollers ; i++) {
+            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)
+                break;
         }
+        
+        if (i == vm->def->ncontrollers) {
+            qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+                             _("Controller does not exist"));
+            return -1;
+        }
+    }
 
-        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);
+    /* 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);
+    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);
+    
     VIR_FREE(safe_path);
     if (ret == -1) {
         virReportOOMError(conn);
@@ -5494,41 +5507,26 @@ try_command:
 
     VIR_FREE(cmd);
 
-    if (controller_specified) {
-        if (qemudParseDriveAddReply(vm, reply, &bus_id, &unit_id) < 0) {
-	    if (!tryOldSyntax && strstr(reply, "invalid char in expression")) {
-	        VIR_FREE(reply);
-		tryOldSyntax = 1;
-		goto try_command;
-	    }
-	    qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
-			      _("adding %s disk failed: %s"), type, reply);
-	    VIR_FREE(reply);
-	    return -1;
-	}
-
-	if (dev->data.disk->bus_id == -1) {
-	    dev->data.disk->bus_id = bus_id;
-	}
-
-	if (dev->data.disk->unit_id == -1) {
-	    dev->data.disk->unit_id = unit_id;
-	}
-    } else {
-        if (qemudParsePciAddReply(vm, reply, &domain, &bus, &slot) < 0) {
-	    if (!tryOldSyntax && strstr(reply, "invalid char in expression")) {
-	        VIR_FREE(reply);
-		tryOldSyntax = 1;
-		goto try_command;
-	    }
-	    
-	    qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
-			      _("adding %s disk failed: %s"), type, reply);
-	    VIR_FREE(reply);
-	    return -1;
-	}
+    if (qemudParseDriveAddReply(vm, reply, &bus_id, &unit_id) < 0) {
+        if (!tryOldSyntax && strstr(reply, "invalid char in expression")) {
+            VIR_FREE(reply);
+            tryOldSyntax = 1;
+            goto try_command;
+        }
+        qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+                          _("adding %s disk failed: %s"), type, reply);
+        VIR_FREE(reply);
+        return -1;
     }
-	
+    
+    if (dev->data.disk->bus_id == -1) {
+        dev->data.disk->bus_id = bus_id;
+    }
+    
+    if (dev->data.disk->unit_id == -1) {
+        dev->data.disk->unit_id = unit_id;
+    }
+    
     dev->data.disk->pci_addr.domain = domain;
     dev->data.disk->pci_addr.bus    = bus;
     dev->data.disk->pci_addr.slot   = slot;
-- 
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]