From 8bac3f7591f7df980a631deeb67a2f20eab6fc62 Mon Sep 17 00:00:00 2001
Message-Id: <8bac3f7591f7df980a631deeb67a2f20eab6fc62@dist-git>
From: Jiri Denemark <jdenemar@redhat.com>
Date: Tue, 5 Apr 2016 09:14:09 +0200
Subject: [PATCH] RHEL: 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.c | 12 ++--
src/qemu/qemu_monitor_json.c | 107 +++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 6 ++
tests/qemuhotplugtest.c | 93 +++++++++++++++++++++++++++++-
4 files changed, 212 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 6fc038a8d9..5e0e95cc51 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2947,8 +2947,10 @@ qemuMonitorDriveDel(qemuMonitorPtr mon,
QEMU_CHECK_MONITOR(mon);
- /* there won't be a direct replacement for drive_del in QMP */
- return qemuMonitorTextDriveDel(mon, drivestr);
+ if (mon->json)
+ return qemuMonitorJSONDriveDel(mon, drivestr);
+ else
+ return qemuMonitorTextDriveDel(mon, drivestr);
}
@@ -3137,8 +3139,10 @@ qemuMonitorAddDrive(qemuMonitorPtr mon,
QEMU_CHECK_MONITOR(mon);
- /* there won't ever be a direct QMP replacement for this function */
- return qemuMonitorTextAddDrive(mon, drivestr);
+ if (mon->json)
+ return qemuMonitorJSONAddDrive(mon, drivestr);
+ else
+ return qemuMonitorTextAddDrive(mon, drivestr);
}
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index af754e870e..ec8469476e 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4055,6 +4055,113 @@ int qemuMonitorJSONDelObject(qemuMonitorPtr mon,
}
+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);
+ cmd = reply = NULL;
+
+ VIR_DEBUG("__com.redhat_drive_add command not found,"
+ " trying upstream way");
+ } else {
+ ret = qemuMonitorJSONCheckError(cmd, reply);
+ goto cleanup;
+ }
+
+ /* Upstream approach */
+ /* there won't be a direct replacement for drive_add in QMP */
+ ret = qemuMonitorTextAddDrive(mon, drivestr);
+
+ cleanup:
+ virJSONValueFree(args);
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+ return ret;
+}
+
+
+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);
+ cmd = reply = NULL;
+
+ 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 */
+ /* there won't be a direct replacement for drive_del in QMP */
+ if ((ret = qemuMonitorTextDriveDel(mon, drivestr)) < 0) {
+ virErrorPtr err = virGetLastError();
+ if (err && err->code == VIR_ERR_OPERATION_UNSUPPORTED) {
+ VIR_ERROR("%s",
+ _("deleting disk is not supported. "
+ "This may leak data if disk is reassigned"));
+ ret = 1;
+ virResetLastError();
+ }
+ }
+
+ cleanup:
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+ return ret;
+}
+
+
int
qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon, virJSONValuePtr actions,
const char *device, const char *file,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index b92fc3762b..9c8fab7cc0 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -238,6 +238,12 @@ int qemuMonitorJSONAddObject(qemuMonitorPtr mon,
int qemuMonitorJSONDelObject(qemuMonitorPtr mon,
const char *objalias);
+int qemuMonitorJSONAddDrive(qemuMonitorPtr mon,
+ const char *drivestr);
+
+int qemuMonitorJSONDriveDel(qemuMonitorPtr mon,
+ const char *drivestr);
+
int qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon,
virJSONValuePtr actions,
const char *device,
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 663e33ed00..cddedf768e 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -665,6 +665,14 @@ mymain(void)
" }" \
"}\r\n"
+#define QMP_NOT_FOUND \
+ "{" \
+ " \"error\": {" \
+ " \"class\": \"CommandNotFound\"," \
+ " \"desc\": \"The command has not been found\"" \
+ " }" \
+ "}"
+
DO_TEST_UPDATE("graphics-spice", "graphics-spice-nochange", false, false, NULL);
DO_TEST_UPDATE("graphics-spice-timeout", "graphics-spice-timeout-nochange", false, false,
"set_password", QMP_OK, "expire_password", QMP_OK);
@@ -685,67 +693,135 @@ mymain(void)
"chardev-remove", QMP_OK);
DO_TEST_ATTACH("base-live", "disk-virtio", false, true,
+ "__com.redhat_drive_add", QMP_NOT_FOUND,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
DO_TEST_DETACH("base-live", "disk-virtio", false, false,
"device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_NOT_FOUND,
"human-monitor-command", HMP(""));
+ DO_TEST_ATTACH("base-live", "disk-virtio", false, true,
+ "__com.redhat_drive_add", QMP_OK,
+ "device_add", QMP_OK);
+ DO_TEST_DETACH("base-live", "disk-virtio", false, false,
+ "device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_OK);
+
DO_TEST_ATTACH_EVENT("base-live", "disk-virtio", false, true,
+ "__com.redhat_drive_add", QMP_NOT_FOUND,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
DO_TEST_DETACH("base-live", "disk-virtio", true, true,
"device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_NOT_FOUND,
"human-monitor-command", HMP(""));
DO_TEST_DETACH("base-live", "disk-virtio", false, false,
"device_del", QMP_DEVICE_DELETED("virtio-disk4") QMP_OK,
+ "__com.redhat_drive_del", QMP_NOT_FOUND,
"human-monitor-command", HMP(""));
+ DO_TEST_ATTACH_EVENT("base-live", "disk-virtio", false, true,
+ "__com.redhat_drive_add", QMP_OK,
+ "device_add", QMP_OK);
+ DO_TEST_DETACH("base-live", "disk-virtio", true, true,
+ "device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_OK);
+ DO_TEST_DETACH("base-live", "disk-virtio", false, false,
+ "device_del", QMP_DEVICE_DELETED("virtio-disk4") QMP_OK,
+ "__com.redhat_drive_del", QMP_OK);
+
DO_TEST_ATTACH("base-live", "disk-usb", false, true,
+ "__com.redhat_drive_add", QMP_NOT_FOUND,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
DO_TEST_DETACH("base-live", "disk-usb", false, false,
"device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_NOT_FOUND,
"human-monitor-command", HMP(""));
+ DO_TEST_ATTACH("base-live", "disk-usb", false, true,
+ "__com.redhat_drive_add", QMP_OK,
+ "device_add", QMP_OK);
+ DO_TEST_DETACH("base-live", "disk-usb", false, false,
+ "device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_OK);
+
DO_TEST_ATTACH_EVENT("base-live", "disk-usb", false, true,
+ "__com.redhat_drive_add", QMP_NOT_FOUND,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
DO_TEST_DETACH("base-live", "disk-usb", true, true,
"device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_NOT_FOUND,
"human-monitor-command", HMP(""));
DO_TEST_DETACH("base-live", "disk-usb", false, false,
"device_del", QMP_DEVICE_DELETED("usb-disk16") QMP_OK,
+ "__com.redhat_drive_del", QMP_NOT_FOUND,
"human-monitor-command", HMP(""));
+ DO_TEST_ATTACH_EVENT("base-live", "disk-usb", false, true,
+ "__com.redhat_drive_add", QMP_OK,
+ "device_add", QMP_OK);
+ DO_TEST_DETACH("base-live", "disk-usb", true, true,
+ "device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_OK);
+ DO_TEST_DETACH("base-live", "disk-usb", false, false,
+ "device_del", QMP_DEVICE_DELETED("usb-disk16") QMP_OK,
+ "__com.redhat_drive_del", QMP_OK);
+
DO_TEST_ATTACH("base-live", "disk-scsi", false, true,
+ "__com.redhat_drive_add", QMP_NOT_FOUND,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
DO_TEST_DETACH("base-live", "disk-scsi", false, false,
"device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_NOT_FOUND,
"human-monitor-command", HMP(""));
+ DO_TEST_ATTACH("base-live", "disk-scsi", false, true,
+ "__com.redhat_drive_add", QMP_OK,
+ "device_add", QMP_OK);
+ DO_TEST_DETACH("base-live", "disk-scsi", false, false,
+ "device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_OK);
+
DO_TEST_ATTACH_EVENT("base-live", "disk-scsi", false, true,
+ "__com.redhat_drive_add", QMP_NOT_FOUND,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
DO_TEST_DETACH("base-live", "disk-scsi", true, true,
"device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_NOT_FOUND,
"human-monitor-command", HMP(""));
DO_TEST_DETACH("base-live", "disk-scsi", false, false,
"device_del", QMP_DEVICE_DELETED("scsi0-0-0-5") QMP_OK,
+ "__com.redhat_drive_del", QMP_NOT_FOUND,
"human-monitor-command", HMP(""));
+ DO_TEST_ATTACH_EVENT("base-live", "disk-scsi", false, true,
+ "__com.redhat_drive_add", QMP_OK,
+ "device_add", QMP_OK);
+ DO_TEST_DETACH("base-live", "disk-scsi", true, true,
+ "device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_OK);
+ DO_TEST_DETACH("base-live", "disk-scsi", false, false,
+ "device_del", QMP_DEVICE_DELETED("scsi0-0-0-5") QMP_OK,
+ "__com.redhat_drive_del", QMP_OK);
+
DO_TEST_ATTACH("base-without-scsi-controller-live", "disk-scsi-2", false, true,
/* Four controllers added */
"device_add", QMP_OK,
"device_add", QMP_OK,
"device_add", QMP_OK,
"device_add", QMP_OK,
- "human-monitor-command", HMP("OK\\r\\n"),
/* Disk added */
+ "__com.redhat_drive_add", QMP_NOT_FOUND,
+ "human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
DO_TEST_DETACH("base-with-scsi-controller-live", "disk-scsi-2", false, false,
"device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_NOT_FOUND,
"human-monitor-command", HMP(""));
DO_TEST_ATTACH_EVENT("base-without-scsi-controller-live", "disk-scsi-2", false, true,
@@ -754,14 +830,17 @@ mymain(void)
"device_add", QMP_OK,
"device_add", QMP_OK,
"device_add", QMP_OK,
- "human-monitor-command", HMP("OK\\r\\n"),
/* Disk added */
+ "__com.redhat_drive_add", QMP_NOT_FOUND,
+ "human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
DO_TEST_DETACH("base-with-scsi-controller-live", "disk-scsi-2", true, true,
"device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_NOT_FOUND,
"human-monitor-command", HMP(""));
DO_TEST_DETACH("base-with-scsi-controller-live", "disk-scsi-2", false, false,
"device_del", QMP_DEVICE_DELETED("scsi3-0-5-7") QMP_OK,
+ "__com.redhat_drive_del", QMP_NOT_FOUND,
"human-monitor-command", HMP(""));
DO_TEST_ATTACH("base-live", "qemu-agent", false, true,
@@ -772,38 +851,47 @@ mymain(void)
"chardev-remove", QMP_OK);
DO_TEST_ATTACH("base-ccw-live", "ccw-virtio", false, true,
+ "__com.redhat_drive_add", QMP_NOT_FOUND,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
DO_TEST_DETACH("base-ccw-live", "ccw-virtio", false, false,
"device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_NOT_FOUND,
"human-monitor-command", HMP(""));
DO_TEST_ATTACH("base-ccw-live-with-ccw-virtio", "ccw-virtio-2", false, true,
+ "__com.redhat_drive_add", QMP_NOT_FOUND,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
DO_TEST_DETACH("base-ccw-live-with-ccw-virtio", "ccw-virtio-2", false, false,
"device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_NOT_FOUND,
"human-monitor-command", HMP(""));
DO_TEST_ATTACH("base-ccw-live-with-ccw-virtio", "ccw-virtio-2-explicit", false, true,
+ "__com.redhat_drive_add", QMP_NOT_FOUND,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
DO_TEST_DETACH("base-ccw-live-with-ccw-virtio", "ccw-virtio-2-explicit", false, false,
"device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_NOT_FOUND,
"human-monitor-command", HMP(""));
/* Attach a second device, then detach the first one. Then attach the first one again. */
DO_TEST_ATTACH("base-ccw-live-with-ccw-virtio", "ccw-virtio-2-explicit", false, true,
+ "__com.redhat_drive_add", QMP_NOT_FOUND,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
DO_TEST_DETACH("base-ccw-live-with-2-ccw-virtio", "ccw-virtio-1-explicit", false, true,
"device_del", QMP_OK,
+ "__com.redhat_drive_del", QMP_NOT_FOUND,
"human-monitor-command", HMP(""));
DO_TEST_ATTACH("base-ccw-live-with-2-ccw-virtio", "ccw-virtio-1-reverse", false, false,
+ "__com.redhat_drive_add", QMP_NOT_FOUND,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
@@ -821,6 +909,7 @@ mymain(void)
"object-del", QMP_OK);
DO_TEST_ATTACH("base-live+disk-scsi-wwn",
"disk-scsi-duplicate-wwn", false, false,
+ "__com.redhat_drive_add", QMP_NOT_FOUND,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
--
2.18.0