Annoyingly when PCI hotplug support was merged from KVM into QEMU the monitor syntax changed :-( This patch tries to deal with this by detecting the error message, and then re-trying with the alternative syntax. It also ensures the monitor error mesages is include with any libvirt error raised, so that we can more easily detect problems in the future FYI, I have a test case for this now in the TCK http://libvirt.org/git/?p=libvirt-tck.git;a=commit;h=0ea2a3f3c426b325f809... Regards, Daniel From: Daniel P. Berrange <berrange(a)redhat.com&gt; Date: Mon, 6 Jul 2009 15:58:55 +0100 Subject: [PATCH 2/3] Fix PCI device hotplug/unplug with newer QEMU * src/qemu_driver.c: If hotplug/unplug fails, try with alternative monitor command syntax --- src/qemu_driver.c | 56 +++++++++++++++++++++++++++++++++++++++------------- 1 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index fdbbb56..25d446d 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3915,6 +3915,7 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn, char *cmd, *reply, *s; char *safe_path; const char* type = virDomainDiskBusTypeToString(dev->data.disk->bus); + int tryAltSyntax = 0; for (i = 0 ; i < vm->def->ndisks ; i++) { if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) { @@ -3929,14 +3930,15 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn, return -1; } +try_command: safe_path = qemudEscapeMonitorArg(dev->data.disk->src); if (!safe_path) { virReportOOMError(conn); return -1; } - ret = virAsprintf(&cmd, "pci_add 0 storage file=%s,if=%s", - safe_path, type); + ret = virAsprintf(&cmd, "pci_add %s storage file=%s,if=%s", + (tryAltSyntax ? "pci_addr=auto" : "0"), safe_path, type); VIR_FREE(safe_path); if (ret == -1) { virReportOOMError(conn); @@ -3952,17 +3954,27 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn, DEBUG ("%s: pci_add reply: %s", vm->def->name, reply); /* If the command succeeds qemu prints: - * OK bus 0... */ -#define PCI_ATTACH_OK_MSG "OK bus 0, slot " - if ((s=strstr(reply, PCI_ATTACH_OK_MSG))) { - char* dummy = s; - s += strlen(PCI_ATTACH_OK_MSG); + * OK bus 0, slot XXX... + * or + * OK domain 0, bus 0, slot XXX + */ + if ((s = strstr(reply, "OK ")) && + (s = strstr(s, "slot "))) { + char *dummy = s; + s += strlen("slot "); if (virStrToLong_i ((const char*)s, &dummy, 10, &dev->data.disk->slotnum) == -1) VIR_WARN("%s", _("Unable to parse slot number\n")); + /* XXX not neccessarily always going to end up in domain 0 / bus 0 :-( */ + /* XXX this slotnum is not persistant across restarts :-( */ + } else if (!tryAltSyntax && strstr(reply, "Invalid pci address")) { + VIR_FREE(reply); + VIR_FREE(cmd); + tryAltSyntax = 1; + goto try_command; } else { qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED, - _("adding %s disk failed"), type); + _("adding %s disk failed: %s"), type, reply); VIR_FREE(reply); VIR_FREE(cmd); return -1; @@ -4179,6 +4191,7 @@ static int qemudDomainDetachPciDiskDevice(virConnectPtr conn, char *cmd = NULL; char *reply = NULL; virDomainDiskDefPtr detach = NULL; + int tryAltSyntax = 0; for (i = 0 ; i < vm->def->ndisks ; i++) { if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) { @@ -4200,9 +4213,17 @@ static int qemudDomainDetachPciDiskDevice(virConnectPtr conn, goto cleanup; } - if (virAsprintf(&cmd, "pci_del 0 %d", detach->slotnum) < 0) { - virReportOOMError(conn); - goto cleanup; +try_command: + if (tryAltSyntax) { + if (virAsprintf(&cmd, "pci_del pci_addr=0:0:%d", detach->slotnum) < 0) { + virReportOOMError(conn); + goto cleanup; + } + } else { + if (virAsprintf(&cmd, "pci_del 0 %d", detach->slotnum) < 0) { + virReportOOMError(conn); + goto cleanup; + } } if (qemudMonitorCommand(vm, cmd, &reply) < 0) { @@ -4212,12 +4233,19 @@ static int qemudDomainDetachPciDiskDevice(virConnectPtr conn, } DEBUG ("%s: pci_del reply: %s",vm->def->name, reply); + + if (!tryAltSyntax && + strstr(reply, "extraneous characters")) { + tryAltSyntax = 1; + goto try_command; + } /* If the command fails due to a wrong slot qemu prints: invalid slot, * nothing is printed on success */ - if (strstr(reply, "invalid slot")) { + if (strstr(reply, "invalid slot") || + strstr(reply, "Invalid pci address")) { qemudReportError (conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("failed to detach disk %s: invalid slot %d"), - detach->dst, detach->slotnum); + _("failed to detach disk %s: invalid slot %d: %s"), + detach->dst, detach->slotnum, reply); goto cleanup; } -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|