You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
456 lines
19 KiB
456 lines
19 KiB
From 7488974ed35b9697199f528357b8ac2ab5623191 Mon Sep 17 00:00:00 2001 |
|
Message-Id: <7488974ed35b9697199f528357b8ac2ab5623191@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 | 106 +++++++++++++++++++++++++++++++++++ |
|
src/qemu/qemu_monitor_json.h | 6 ++ |
|
tests/qemuhotplugtest.c | 93 +++++++++++++++++++++++++++++- |
|
4 files changed, 211 insertions(+), 6 deletions(-) |
|
|
|
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c |
|
index deb2de83fa..f525d60cf0 100644 |
|
--- a/src/qemu/qemu_monitor.c |
|
+++ b/src/qemu/qemu_monitor.c |
|
@@ -3163,8 +3163,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); |
|
} |
|
|
|
|
|
@@ -3285,8 +3287,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 5cc83cad36..908e29eac3 100644 |
|
--- a/src/qemu/qemu_monitor_json.c |
|
+++ b/src/qemu/qemu_monitor_json.c |
|
@@ -4000,6 +4000,112 @@ 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 qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, |
|
const char *alias, |
|
const char *passphrase) |
|
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h |
|
index 99815006d7..580d810829 100644 |
|
--- a/src/qemu/qemu_monitor_json.h |
|
+++ b/src/qemu/qemu_monitor_json.h |
|
@@ -237,6 +237,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 qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, |
|
const char *alias, |
|
const char *passphrase); |
|
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c |
|
index 85e53653e1..4757e0b6b3 100644 |
|
--- a/tests/qemuhotplugtest.c |
|
+++ b/tests/qemuhotplugtest.c |
|
@@ -664,6 +664,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); |
|
@@ -684,67 +692,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, |
|
@@ -753,14 +829,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, |
|
@@ -771,38 +850,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); |
|
|
|
@@ -820,6 +908,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.17.0 |
|
|
|
|