7a3408
From b32e593db092c9c4f7c85028fe318f6b401451cb Mon Sep 17 00:00:00 2001
7a3408
Message-Id: <b32e593db092c9c4f7c85028fe318f6b401451cb@dist-git>
c401cc
From: "Daniel P. Berrange" <berrange@redhat.com>
c401cc
Date: Fri, 17 Dec 2010 14:55:38 +0100
9119d9
Subject: [PATCH] RHEL: Support virtio disk hotplug in JSON mode
c401cc
c401cc
RHEL only, no upstream
c401cc
c401cc
For bug
c401cc
  https://bugzilla.redhat.com/show_bug.cgi?id=1026966
c401cc
  https://bugzilla.redhat.com/show_bug.cgi?id=573946
c401cc
c401cc
The existing drive_add command can hotplug SCSI and VirtIO
c401cc
disks, but this isn't ported to JSON mode. RHEL6 introduces
c401cc
a custom __com.redhat_drive_add that only supports VirtIO
c401cc
disks. Switch the VirtIO hotplug to this command, but leave
c401cc
the SCSI hotplug using old command so SCSI gets an explicit
c401cc
error about being unsupported.
c401cc
c401cc
* src/libvirt_private.syms: Export virJSONValueObjectRemoveKey
c401cc
* src/util/json.c, src/util/json.h: Add virJSONValueObjectRemoveKey
c401cc
  to allow a key to be deleted from an object
c401cc
* src/qemu/qemu_monitor_json.c: Try __com.redhat_drive_add first and use
c401cc
  drive_add only if the redhat command is not known to qemu.
c401cc
c401cc
Also includes the following fix:
c401cc
c401cc
https://bugzilla.redhat.com/show_bug.cgi?id=696596
c401cc
c401cc
Upstream added drive_del as a way to ensure that disks are fully
c401cc
removed before returning control to libvirt.  But RHEL backported
c401cc
it as __com.redhat_drive_del, prior to upstream adoption of a
c401cc
QMP counterpart.  Because we weren't trying the RHEL-specific
c401cc
spelling, we were falling back to the unsafe approach of just
c401cc
removing the device and hoping for the best, which was racy and
c401cc
could occasionally result in a rapid hot-plug cycle trying to
c401cc
plug in a new disk that collides with the old disk not yet gone.
c401cc
c401cc
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONDriveDel): Try
c401cc
rhel-specific drive_del monitor command first.
c401cc
c401cc
(cherry picked from commit d1c200dfead14a590a4ddebe20a20ffe441d2b24 in
c401cc
rhel-6.5 branch)
c401cc
c401cc
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
c401cc
c401cc
Conflicts:
c401cc
	src/libvirt_private.syms - the change is already upstream
c401cc
        src/util/virjson.[ch] - the change is already upstream
c401cc
	src/qemu/qemu_monitor_json.c - context; upstream doesn't try to
c401cc
	    call nonexistent drive_{add,del} QMP commands any more
c401cc
---
9119d9
 src/qemu/qemu_monitor_json.c | 81 +++++++++++++++++++++++++++++++++++++++++++-
9119d9
 tests/qemuhotplugtest.c      | 74 ++++++++++++++++++++++++++++++++++++++++
9119d9
 2 files changed, 154 insertions(+), 1 deletion(-)
c401cc
c401cc
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
7a3408
index edfa67f..fd567e4 100644
c401cc
--- a/src/qemu/qemu_monitor_json.c
c401cc
+++ b/src/qemu/qemu_monitor_json.c
7a3408
@@ -3474,9 +3474,54 @@ int qemuMonitorJSONDelObject(qemuMonitorPtr mon,
c401cc
 int qemuMonitorJSONAddDrive(qemuMonitorPtr mon,
c401cc
                             const char *drivestr)
c401cc
 {
c401cc
+    int ret = -1;
c401cc
+    virJSONValuePtr cmd;
c401cc
+    virJSONValuePtr reply = NULL;
c401cc
+    virJSONValuePtr args;
c401cc
+
c401cc
+    cmd = qemuMonitorJSONMakeCommand("__com.redhat_drive_add",
c401cc
+                                     NULL);
c401cc
+    if (!cmd)
c401cc
+        return -1;
c401cc
+
c401cc
+    args = qemuMonitorJSONKeywordStringToJSON(drivestr, "type");
c401cc
+    if (!args)
c401cc
+        goto cleanup;
c401cc
+
c401cc
+    /* __com.redhat_drive_add rejects the 'if' key */
c401cc
+    virJSONValueObjectRemoveKey(args, "if", NULL);
c401cc
+
c401cc
+    if (virJSONValueObjectAppend(cmd, "arguments", args) < 0) {
c401cc
+        virReportOOMError();
c401cc
+        goto cleanup;
c401cc
+    }
c401cc
+    args = NULL; /* cmd owns reference to args now */
c401cc
+
9119d9
+    if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
c401cc
+        goto cleanup;
c401cc
+
c401cc
+    if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
c401cc
+        virJSONValueFree(cmd);
c401cc
+        virJSONValueFree(reply);
9119d9
+        cmd = reply = NULL;
c401cc
+
c401cc
+        VIR_DEBUG("__com.redhat_drive_add command not found,"
9119d9
+                  " trying upstream way");
c401cc
+    } else {
c401cc
+        ret = qemuMonitorJSONCheckError(cmd, reply);
c401cc
+        goto cleanup;
c401cc
+    }
c401cc
+
c401cc
+    /* Upstream approach */
c401cc
     /* XXX Update to use QMP, if QMP ever adds support for drive_add */
c401cc
     VIR_DEBUG("drive_add command not found, trying HMP");
c401cc
-    return qemuMonitorTextAddDrive(mon, drivestr);
c401cc
+    ret = qemuMonitorTextAddDrive(mon, drivestr);
c401cc
+
c401cc
+cleanup:
c401cc
+    virJSONValueFree(args);
c401cc
+    virJSONValueFree(cmd);
c401cc
+    virJSONValueFree(reply);
c401cc
+    return ret;
c401cc
 }
c401cc
 
c401cc
 
7a3408
@@ -3484,7 +3529,37 @@ int qemuMonitorJSONDriveDel(qemuMonitorPtr mon,
c401cc
                             const char *drivestr)
c401cc
 {
c401cc
     int ret;
c401cc
+    virJSONValuePtr cmd;
c401cc
+    virJSONValuePtr reply = NULL;
9119d9
 
c401cc
+    VIR_DEBUG("drivestr=%s", drivestr);
c401cc
+    cmd = qemuMonitorJSONMakeCommand("__com.redhat_drive_del",
c401cc
+                                     "s:id", drivestr,
c401cc
+                                     NULL);
c401cc
+    if (!cmd)
c401cc
+        return -1;
9119d9
+
c401cc
+    if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
c401cc
+        goto cleanup;
c401cc
+
c401cc
+    if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
c401cc
+        virJSONValueFree(cmd);
c401cc
+        virJSONValueFree(reply);
9119d9
+        cmd = reply = NULL;
c401cc
+
c401cc
+        VIR_DEBUG("__com.redhat_drive_del command not found,"
9119d9
+                  " trying upstream way");
c401cc
+    } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
c401cc
+        /* NB: device not found errors mean the drive was
c401cc
+         * auto-deleted and we ignore the error */
c401cc
+        ret = 0;
c401cc
+        goto cleanup;
c401cc
+    } else {
c401cc
+        ret = qemuMonitorJSONCheckError(cmd, reply);
c401cc
+        goto cleanup;
c401cc
+    }
c401cc
+
c401cc
+    /* Upstream approach */
c401cc
     /* XXX Update to use QMP, if QMP ever adds support for drive_del */
c401cc
     VIR_DEBUG("drive_del command not found, trying HMP");
c401cc
     if ((ret = qemuMonitorTextDriveDel(mon, drivestr)) < 0) {
7a3408
@@ -3497,6 +3572,10 @@ int qemuMonitorJSONDriveDel(qemuMonitorPtr mon,
c401cc
             virResetLastError();
c401cc
         }
c401cc
     }
c401cc
+
c401cc
+cleanup:
c401cc
+    virJSONValueFree(cmd);
c401cc
+    virJSONValueFree(reply);
c401cc
     return ret;
c401cc
 }
c401cc
 
9119d9
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
7a3408
index 368a5e7..3b547f2 100644
9119d9
--- a/tests/qemuhotplugtest.c
9119d9
+++ b/tests/qemuhotplugtest.c
7a3408
@@ -416,6 +416,14 @@ mymain(void)
9119d9
     "    }"                                                 \
9119d9
     "}\r\n"
9119d9
 
9119d9
+#define QMP_NOT_FOUND \
9119d9
+    "{" \
9119d9
+    "    \"error\": {" \
9119d9
+    "        \"class\": \"CommandNotFound\"," \
9119d9
+    "        \"desc\": \"The command has not been found\"" \
9119d9
+    "    }" \
9119d9
+    "}"
9119d9
+
9119d9
     DO_TEST_UPDATE("graphics-spice", "graphics-spice-nochange", false, false, NULL);
9119d9
     DO_TEST_UPDATE("graphics-spice-timeout", "graphics-spice-timeout-nochange", false, false,
9119d9
                    "set_password", QMP_OK, "expire_password", QMP_OK);
7a3408
@@ -436,56 +444,122 @@ mymain(void)
9119d9
                    "chardev-remove", QMP_OK);
9119d9
 
9119d9
     DO_TEST_ATTACH("hotplug-base", "disk-virtio", false, true,
9119d9
+                   "__com.redhat_drive_add", QMP_NOT_FOUND,
9119d9
                    "human-monitor-command", HMP("OK\\r\\n"),
9119d9
                    "device_add", QMP_OK);
9119d9
     DO_TEST_DETACH("hotplug-base", "disk-virtio", false, false,
9119d9
                    "device_del", QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_NOT_FOUND,
9119d9
                    "human-monitor-command", HMP(""));
9119d9
 
9119d9
+    DO_TEST_ATTACH("hotplug-base", "disk-virtio", false, true,
9119d9
+                   "__com.redhat_drive_add", QMP_OK,
9119d9
+                   "device_add", QMP_OK);
9119d9
+    DO_TEST_DETACH("hotplug-base", "disk-virtio", false, false,
9119d9
+                   "device_del", QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_OK);
9119d9
+
9119d9
     DO_TEST_ATTACH_EVENT("hotplug-base", "disk-virtio", false, true,
9119d9
+                         "__com.redhat_drive_add", QMP_NOT_FOUND,
9119d9
                          "human-monitor-command", HMP("OK\\r\\n"),
9119d9
                          "device_add", QMP_OK);
9119d9
     DO_TEST_DETACH("hotplug-base", "disk-virtio", true, true,
9119d9
                    "device_del", QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_NOT_FOUND,
9119d9
                    "human-monitor-command", HMP(""));
9119d9
     DO_TEST_DETACH("hotplug-base", "disk-virtio", false, false,
9119d9
                    "device_del", QMP_DEVICE_DELETED("virtio-disk4") QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_NOT_FOUND,
9119d9
                    "human-monitor-command", HMP(""));
9119d9
 
9119d9
+    DO_TEST_ATTACH_EVENT("hotplug-base", "disk-virtio", false, true,
9119d9
+                         "__com.redhat_drive_add", QMP_OK,
9119d9
+                         "device_add", QMP_OK);
9119d9
+    DO_TEST_DETACH("hotplug-base", "disk-virtio", true, true,
9119d9
+                   "device_del", QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_OK);
9119d9
+    DO_TEST_DETACH("hotplug-base", "disk-virtio", false, false,
9119d9
+                   "device_del", QMP_DEVICE_DELETED("virtio-disk4") QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_OK);
9119d9
+
9119d9
     DO_TEST_ATTACH("hotplug-base", "disk-usb", false, true,
9119d9
+                   "__com.redhat_drive_add", QMP_NOT_FOUND,
9119d9
                    "human-monitor-command", HMP("OK\\r\\n"),
9119d9
                    "device_add", QMP_OK);
9119d9
     DO_TEST_DETACH("hotplug-base", "disk-usb", false, false,
9119d9
                    "device_del", QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_NOT_FOUND,
9119d9
                    "human-monitor-command", HMP(""));
9119d9
 
9119d9
+    DO_TEST_ATTACH("hotplug-base", "disk-usb", false, true,
9119d9
+                   "__com.redhat_drive_add", QMP_OK,
9119d9
+                   "device_add", QMP_OK);
9119d9
+    DO_TEST_DETACH("hotplug-base", "disk-usb", false, false,
9119d9
+                   "device_del", QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_OK);
9119d9
+
9119d9
     DO_TEST_ATTACH_EVENT("hotplug-base", "disk-usb", false, true,
9119d9
+                         "__com.redhat_drive_add", QMP_NOT_FOUND,
9119d9
                          "human-monitor-command", HMP("OK\\r\\n"),
9119d9
                          "device_add", QMP_OK);
9119d9
     DO_TEST_DETACH("hotplug-base", "disk-usb", true, true,
9119d9
                    "device_del", QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_NOT_FOUND,
9119d9
                    "human-monitor-command", HMP(""));
9119d9
     DO_TEST_DETACH("hotplug-base", "disk-usb", false, false,
9119d9
                    "device_del", QMP_DEVICE_DELETED("usb-disk16") QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_NOT_FOUND,
9119d9
                    "human-monitor-command", HMP(""));
9119d9
 
9119d9
+    DO_TEST_ATTACH_EVENT("hotplug-base", "disk-usb", false, true,
9119d9
+                         "__com.redhat_drive_add", QMP_OK,
9119d9
+                         "device_add", QMP_OK);
9119d9
+    DO_TEST_DETACH("hotplug-base", "disk-usb", true, true,
9119d9
+                   "device_del", QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_OK);
9119d9
+    DO_TEST_DETACH("hotplug-base", "disk-usb", false, false,
9119d9
+                   "device_del", QMP_DEVICE_DELETED("usb-disk16") QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_OK);
9119d9
+
9119d9
     DO_TEST_ATTACH("hotplug-base", "disk-scsi", false, true,
9119d9
+                   "__com.redhat_drive_add", QMP_NOT_FOUND,
9119d9
                    "human-monitor-command", HMP("OK\\r\\n"),
9119d9
                    "device_add", QMP_OK);
9119d9
     DO_TEST_DETACH("hotplug-base", "disk-scsi", false, false,
9119d9
                    "device_del", QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_NOT_FOUND,
9119d9
                    "human-monitor-command", HMP(""));
9119d9
 
9119d9
+    DO_TEST_ATTACH("hotplug-base", "disk-scsi", false, true,
9119d9
+                   "__com.redhat_drive_add", QMP_OK,
9119d9
+                   "device_add", QMP_OK);
9119d9
+    DO_TEST_DETACH("hotplug-base", "disk-scsi", false, false,
9119d9
+                   "device_del", QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_OK);
9119d9
+
9119d9
     DO_TEST_ATTACH_EVENT("hotplug-base", "disk-scsi", false, true,
9119d9
+                         "__com.redhat_drive_add", QMP_NOT_FOUND,
9119d9
                          "human-monitor-command", HMP("OK\\r\\n"),
9119d9
                          "device_add", QMP_OK);
9119d9
     DO_TEST_DETACH("hotplug-base", "disk-scsi", true, true,
9119d9
                    "device_del", QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_NOT_FOUND,
9119d9
                    "human-monitor-command", HMP(""));
9119d9
     DO_TEST_DETACH("hotplug-base", "disk-scsi", false, false,
9119d9
                    "device_del", QMP_DEVICE_DELETED("scsi0-0-0-5") QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_NOT_FOUND,
9119d9
                    "human-monitor-command", HMP(""));
9119d9
 
9119d9
+    DO_TEST_ATTACH_EVENT("hotplug-base", "disk-scsi", false, true,
9119d9
+                         "__com.redhat_drive_add", QMP_OK,
9119d9
+                         "device_add", QMP_OK);
9119d9
+    DO_TEST_DETACH("hotplug-base", "disk-scsi", true, true,
9119d9
+                   "device_del", QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_OK);
9119d9
+    DO_TEST_DETACH("hotplug-base", "disk-scsi", false, false,
9119d9
+                   "device_del", QMP_DEVICE_DELETED("scsi0-0-0-5") QMP_OK,
9119d9
+                   "__com.redhat_drive_del", QMP_OK);
9119d9
+
9119d9
     virObjectUnref(driver.caps);
9119d9
     virObjectUnref(driver.xmlopt);
9119d9
     virObjectUnref(driver.config);
c401cc
-- 
7a3408
2.4.5
c401cc