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

We need this multiple times later on. Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer(a)siemens.com&gt; Signed-off-by: Jan Kiszka <jan.kiszka(a)siemens.com&gt; --- 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