Re: [libvirt] [PATCH 11/34] Properly support SCSI drive hotplug

The current SCSI hotplug support attaches a brand new SCSI controller for every disk. This is broken because the semantics differ from those used when starting the VM initially. In the latter case, each SCSI controller is filled before a new one is added. If the user specifies an high drive index (sdazz) then at initial startup, many intermediate SCSI controllers may be added with no drives.

This patch changes SCSI hotplug so that it exactly matches the behaviour of initial startup. First the SCSI controller number is determined for the drive to be hotplugged. If any controller upto and including that controller number is not yet present, it is attached. Then finally the drive is attached to the last controller. NB, this breaks SCSI hotunplug, because there is no 'drive_del' command in current QEMU. Previous SCSI hotunplug was broken in any case because it was unplugging the entire controller, not just the drive in question. A future QEMU will allow proper SCSI hotunplug of a drive. This patch is derived from work done by Wolfgang Mauerer on disk controllers. * src/qemu/qemu_driver.c: Fix SCSI hotplug to add a drive to the correct controller, instead of just attaching a new controller. * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h, src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h, src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Add support for 'drive_add' command --- src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 123 +++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_monitor.c | 20 +++++++ src/qemu/qemu_monitor.h | 5 ++ src/qemu/qemu_monitor_json.c | 81 +++++++++++++++++++++++++--- src/qemu/qemu_monitor_json.h | 5 ++ src/qemu/qemu_monitor_text.c | 102 ++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 5 ++ 8 files changed, 329 insertions(+), 13 deletions(-)

+static virDomainControllerDefPtr +qemuDomainFindOrCreateSCSIDiskController(virConnectPtr conn, + struct qemud_driver *driver, + virDomainObjPtr vm, + int controller) +{ + int i; + virDomainControllerDefPtr cont; + for (i = 0 ; i < vm->def->ncontrollers ; i++) { + cont = vm->def->controllers[i]; + + if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) + continue; + + if (cont->idx == controller) + return cont; + } + + /* No SCSI controller present, for back compatability we + * now hotplug a controller */ + if (VIR_ALLOC(cont) < 0) { + virReportOOMError(conn); + return NULL; + } + cont->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI; + cont->idx = 0; + + VIR_INFO0("No SCSI controller present, hotplugging one"); + if (qemudDomainAttachPciControllerDevice(conn, driver, + vm, cont) < 0) { + VIR_FREE(cont); + return NULL; + } + return cont; +}

@@ -1515,7 +1514,75 @@ int qemuMonitorJSONAttachPCIDiskController(qemuMonitorPtr mon, ret = qemuMonitorJSONCheckError(cmd, reply); if (ret == 0 && - qemuMonitorJSONGetGuestAddress(reply, guestAddr) < 0) + qemuMonitorJSONGetGuestPCIAddress(reply, guestAddr) < 0) + ret = -1; + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +}

+ if (ret == 0 && + qemuMonitorJSONGetGuestDriveAddress(reply, driveAddr) < 0) ret = -1; virJSONValueFree(cmd);

+static int +qemudParseDriveAddReply(const char *reply, + virDomainDeviceDriveAddressPtr addr) +{ + char *s, *e; + + /* If the command succeeds qemu prints: + * OK bus X, unit Y + */ + + if (!(s = strstr(reply, "OK "))) + return -1; + + s += 3;

+ if (STRPREFIX(s, "bus ")) { + s += strlen("bus "); + + if (virStrToLong_ui(s, &e, 10, &addr->bus) == -1) { + VIR_WARN(_("Unable to parse bus '%s'\n"), s); + return -1; + } + + if (!STRPREFIX(e, ", ")) { + VIR_WARN(_("Expected ', ' parsing drive_add reply '%s'\n"), s); + return -1; + } + s = e + 2; + }

+ if (!STRPREFIX(s, "unit ")) { + VIR_WARN(_("Expected 'unit ' parsing drive_add reply '%s'\n"), s); + return -1; + } + s += strlen("bus "); + + if (virStrToLong_ui(s, &e, 10, &addr->unit) == -1) { + VIR_WARN(_("Unable to parse unit number '%s'\n"), s); + return -1; + } + + return 0; +}