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

From: Wolfgang Mauerer <wolfgang.mauerer(a)siemens.com&gt; 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(a)siemens.com&gt; Signed-off-by: Jan Kiszka <jan.kiszka(a)siemens.com&gt; --- 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