From 673fabafb0dc8eca749e9b0eae13645afe752e42 Mon Sep 17 00:00:00 2001
Message-Id: <673fabafb0dc8eca749e9b0eae13645afe752e42.1386932213.git.jdenemar@redhat.com>
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Fri, 17 Dec 2010 14:55:38 +0100
Subject: [PATCH] Support virtio disk hotplug in JSON mode

RHEL only, no upstream

For bug
  https://bugzilla.redhat.com/show_bug.cgi?id=1026966
  https://bugzilla.redhat.com/show_bug.cgi?id=573946

The existing drive_add command can hotplug SCSI and VirtIO
disks, but this isn't ported to JSON mode. RHEL6 introduces
a custom __com.redhat_drive_add that only supports VirtIO
disks. Switch the VirtIO hotplug to this command, but leave
the SCSI hotplug using old command so SCSI gets an explicit
error about being unsupported.

* src/libvirt_private.syms: Export virJSONValueObjectRemoveKey
* src/util/json.c, src/util/json.h: Add virJSONValueObjectRemoveKey
  to allow a key to be deleted from an object
* src/qemu/qemu_monitor_json.c: Try __com.redhat_drive_add first and use
  drive_add only if the redhat command is not known to qemu.

Also includes the following fix:

https://bugzilla.redhat.com/show_bug.cgi?id=696596

Upstream added drive_del as a way to ensure that disks are fully
removed before returning control to libvirt.  But RHEL backported
it as __com.redhat_drive_del, prior to upstream adoption of a
QMP counterpart.  Because we weren't trying the RHEL-specific
spelling, we were falling back to the unsafe approach of just
removing the device and hoping for the best, which was racy and
could occasionally result in a rapid hot-plug cycle trying to
plug in a new disk that collides with the old disk not yet gone.

* src/qemu/qemu_monitor_json.c (qemuMonitorJSONDriveDel): Try
rhel-specific drive_del monitor command first.

(cherry picked from commit d1c200dfead14a590a4ddebe20a20ffe441d2b24 in
rhel-6.5 branch)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

Conflicts:
	src/libvirt_private.syms - the change is already upstream
        src/util/virjson.[ch] - the change is already upstream
	src/qemu/qemu_monitor_json.c - context; upstream doesn't try to
	    call nonexistent drive_{add,del} QMP commands any more
---
 src/qemu/qemu_monitor_json.c | 79 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 78 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 18497e3..7b7992f 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3113,9 +3113,53 @@ cleanup:
 int qemuMonitorJSONAddDrive(qemuMonitorPtr mon,
                             const char *drivestr)
 {
+    int ret = -1;
+    virJSONValuePtr cmd;
+    virJSONValuePtr reply = NULL;
+    virJSONValuePtr args;
+
+    cmd = qemuMonitorJSONMakeCommand("__com.redhat_drive_add",
+                                     NULL);
+    if (!cmd)
+        return -1;
+
+    args = qemuMonitorJSONKeywordStringToJSON(drivestr, "type");
+    if (!args)
+        goto cleanup;
+
+    /* __com.redhat_drive_add rejects the 'if' key */
+    virJSONValueObjectRemoveKey(args, "if", NULL);
+
+    if (virJSONValueObjectAppend(cmd, "arguments", args) < 0) {
+        virReportOOMError();
+        goto cleanup;
+    }
+    args = NULL; /* cmd owns reference to args now */
+
+    if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply) < 0))
+        goto cleanup;
+
+    if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+        virJSONValueFree(cmd);
+        virJSONValueFree(reply);
+
+        VIR_DEBUG("__com.redhat_drive_add command not found,"
+                   " trying upstream way");
+    } else {
+        ret = qemuMonitorJSONCheckError(cmd, reply);
+        goto cleanup;
+    }
+
+    /* Upstream approach */
     /* XXX Update to use QMP, if QMP ever adds support for drive_add */
     VIR_DEBUG("drive_add command not found, trying HMP");
-    return qemuMonitorTextAddDrive(mon, drivestr);
+    ret = qemuMonitorTextAddDrive(mon, drivestr);
+
+cleanup:
+    virJSONValueFree(args);
+    virJSONValueFree(cmd);
+    virJSONValueFree(reply);
+    return ret;
 }
 
 
@@ -3123,7 +3167,36 @@ int qemuMonitorJSONDriveDel(qemuMonitorPtr mon,
                             const char *drivestr)
 {
     int ret;
+    virJSONValuePtr cmd;
+    virJSONValuePtr reply = NULL;
+
+    VIR_DEBUG("drivestr=%s", drivestr);
+    cmd = qemuMonitorJSONMakeCommand("__com.redhat_drive_del",
+                                     "s:id", drivestr,
+                                     NULL);
+    if (!cmd)
+        return -1;
 
+    if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
+        goto cleanup;
+
+    if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+        virJSONValueFree(cmd);
+        virJSONValueFree(reply);
+
+        VIR_DEBUG("__com.redhat_drive_del command not found,"
+                   " trying upstream way");
+    } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
+        /* NB: device not found errors mean the drive was
+         * auto-deleted and we ignore the error */
+        ret = 0;
+        goto cleanup;
+    } else {
+        ret = qemuMonitorJSONCheckError(cmd, reply);
+        goto cleanup;
+    }
+
+    /* Upstream approach */
     /* XXX Update to use QMP, if QMP ever adds support for drive_del */
     VIR_DEBUG("drive_del command not found, trying HMP");
     if ((ret = qemuMonitorTextDriveDel(mon, drivestr)) < 0) {
@@ -3136,6 +3209,10 @@ int qemuMonitorJSONDriveDel(qemuMonitorPtr mon,
             virResetLastError();
         }
     }
+
+cleanup:
+    virJSONValueFree(cmd);
+    virJSONValueFree(reply);
     return ret;
 }
 
-- 
1.8.5.1