[libvirt] PATCH: Handle changed monitor syntax for PCI hotplug/unplug

Daniel P. Berrange berrange at redhat.com
Tue Jul 7 16:45:58 UTC 2009


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=0ea2a3f3c426b325f809813a9584ab3a9aa33ce6

Regards,
Daniel

>From fca1582a7406d9eef7ff1a5e108986df76aeb6a1 Mon Sep 17 00:00:00 2001
From: Daniel P. Berrange <berrange at redhat.com>
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 :|




More information about the libvir-list mailing list