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