Re: [libvirt] [PATCH 9/9] Implement disk- and controller hotremove

Since both disks and disk controllers can be dynamically added to the system, it makes sense to be also able to remove them.

diff --git a/src/domain_conf.h b/src/domain_conf.h index 17f8b14..c7d49cf 100644 --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -149,6 +149,14 @@ virDiskHasValidPciAddr(virDomainDiskDefPtr def) return def->pci_addr.domain || def->pci_addr.domain || def->pci_addr.slot; } +static inline int +virDiskHasValidController(virDomainDiskDefPtr def) +{ + return def->controller_id != NULL || + def->controller_pci_addr.domain || def->controller_pci_addr.domain + || def->controller_pci_addr.slot; +} + /* Two types of disk backends */ enum virDomainFSType { diff --git a/src/qemu_driver.c b/src/qemu_driver.c index ddc46f6..5ac89a1 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -6204,32 +6204,31 @@ cleanup: return ret; } -static int qemudDomainDetachPciDiskDevice(virConnectPtr conn, - virDomainObjPtr vm, virDomainDeviceDefPtr dev) +static int qemudDomainDetachDiskController(virConnectPtr conn, + virDomainObjPtr vm, virDomainDeviceDefPtr dev) { int i, ret = -1; char *cmd = NULL; char *reply = NULL; - virDomainDiskDefPtr detach = NULL; + virDomainControllerDefPtr detach = NULL; int tryOldSyntax = 0; - for (i = 0 ; i < vm->def->ndisks ; i++) { - if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) { - detach = vm->def->disks[i]; +// virDomainControllerDefPtr is dev->data.controller + for (i = 0 ; i < vm->def->ncontrollers ; i++) { + if (vm->def->controllers[i]->pci_addr.domain == + dev->data.controller->pci_addr.domain && + vm->def->controllers[i]->pci_addr.bus == + dev->data.controller->pci_addr.bus && + vm->def->controllers[i]->pci_addr.slot == + dev->data.controller->pci_addr.slot) { + detach = vm->def->controllers[i]; break; } } if (!detach) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("disk %s not found"), dev->data.disk->dst); - goto cleanup; - } - - if (!virDiskHasValidPciAddr(detach)) { - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("disk %s cannot be detached - no PCI address for device"), - detach->dst); + _("Controller %s not found"), dev->data.disk->dst); goto cleanup; } @@ -6251,7 +6250,9 @@ try_command: if (qemudMonitorCommand(vm, cmd, &reply) < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("failed to execute detach disk %s command"), detach->dst); + _("failed to execute detach controller %d:%d:%d " \ + "command"), detach->pci_addr.domain, + detach->pci_addr.bus, detach->pci_addr.slot); goto cleanup; } @@ -6267,6 +6268,124 @@ try_command: if (strstr(reply, "invalid slot") || strstr(reply, "Invalid pci address")) { qemudReportError (conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("failed to detach controller: invalid PCI address %.4x:%.2x:%.2x: %s"), + detach->pci_addr.domain, + detach->pci_addr.bus, + detach->pci_addr.slot, + reply); + goto cleanup; + } + + if (vm->def->ncontrollers > 1) { + vm->def->controllers[i] = vm->def->controllers[--vm->def->ncontrollers]; + if (VIR_REALLOC_N(vm->def->controllers, vm->def->ncontrollers) < 0) { + virReportOOMError(conn); + goto cleanup; + } + qsort(vm->def->disks, vm->def->ncontrollers, + sizeof(*vm->def->controllers), + virDomainControllerQSort); + } else { + VIR_FREE(vm->def->controllers[0]); + vm->def->controllers = 0; + } + ret = 0; + +cleanup: + VIR_FREE(reply); + VIR_FREE(cmd); + return ret; +} + +static int qemudDomainDetachDiskDevice(virConnectPtr conn, + virDomainObjPtr vm, virDomainDeviceDefPtr dev) +{ + int i, ret = -1; + char *cmd = NULL; + char *reply = NULL; + const char* type; + virDomainDiskDefPtr detach = NULL; + int tryOldSyntax = 0; + + /* If bus and unit are specified, use these first to identify + the disk */ + if (dev->data.disk->controller_pci_addr.domain != -1) { + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (dev->data.disk->bus_id != -1 && + dev->data.disk->bus_id == vm->def->disks[i]->bus_id && + dev->data.disk->unit_id != -1 && + dev->data.disk->unit_id == vm->def->disks[i]->unit_id) { + detach = vm->def->disks[i]; + break; + } + } + } + + /* If the device did not specify a controller explicitely (and therefore + lacks a bus and unit specification), revert to the explicit target + device specification to identify a device for removal. */ + if (!detach) { + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) { + detach = vm->def->disks[i]; + break; + } + } + } + + if (!detach) { + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("disk %s not found"), dev->data.disk->dst); + goto cleanup; + } + + if (!virDiskHasValidPciAddr(detach) && !virDiskHasValidController(detach)) { + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("disk %s cannot be detached - no PCI address for device"), + detach->dst); + goto cleanup; + } + + type = virDomainDiskBusTypeToString(detach->bus); + +try_command: + if (tryOldSyntax) { + if (virAsprintf(&cmd, "drive_del 0 %d:%d:%d bus=%d,unit=%d,if=%s", + detach->pci_addr.domain, detach->pci_addr.bus, + detach->pci_addr.slot, detach->bus_id, + detach->unit_id, type) < 0) { + virReportOOMError(conn); + goto cleanup; + } + } else { + if (virAsprintf(&cmd, "drive_del pci_addr=%d:%d:%d bus=%d,unit=%d,if=%s", + detach->pci_addr.domain, + detach->pci_addr.bus, + detach->pci_addr.slot, detach->bus_id, + detach->unit_id, type) < 0) { + virReportOOMError(conn); + goto cleanup; + } + } + + if (qemudMonitorCommand(vm, cmd, &reply) < 0) { + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("failed to execute detach disk %s command"), detach->dst); + goto cleanup; + } + + DEBUG ("%s: drive_del reply: %s",vm->def->name, reply); + + if (!tryOldSyntax && + strstr(reply, "extraneous characters")) { + tryOldSyntax = 1; + goto try_command; + } + /* OK bux x, unit x is printed on success, but this does not provide + any new information to us.*/ + if (strstr(reply, "Invalid pci address") || + strstr(reply, "No pci device with address")) { + qemudReportError (conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, _("failed to detach disk %s: invalid PCI address %.4x:%.2x:%.2x: %s"), detach->dst, detach->pci_addr.domain, @@ -6274,6 +6393,11 @@ try_command: detach->pci_addr.slot, reply); goto cleanup; + } else if (strstr(reply, "Need to specify bus") || + strstr(reply, "Need to specify unit")) { + qemudReportError (conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("failed to detach disk %s: bus and unit not (internal error)"), + detach->dst); } if (vm->def->ndisks > 1) { @@ -6575,7 +6699,7 @@ static int qemudDomainDetachDevice(virDomainPtr dom, dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK && (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI || dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO)) { - ret = qemudDomainDetachPciDiskDevice(dom->conn, vm, dev); + ret = qemudDomainDetachDiskDevice(dom->conn, vm, dev); if (driver->securityDriver) driver->securityDriver->domainRestoreSecurityImageLabel(dom->conn, dev->data.disk); if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 1) < 0) @@ -6584,6 +6708,11 @@ static int qemudDomainDetachDevice(virDomainPtr dom, ret = qemudDomainDetachNetDevice(dom->conn, vm, dev); } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { ret = qemudDomainDetachHostDevice(dom->conn, driver, vm, dev); + } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { + /* NOTE: We can unplug the controller without having to + check if all disks are unplugged; it is at the user's + responsibility to use the required OS services first. */ + ret = qemudDomainDetachDiskController(dom->conn, vm, dev);