On Fri, Mar 11, 2011 at 04:05:16PM +0100, Jiri Denemark wrote:
> > - /* See if drive_del isn't supported */
> > - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
> > - VIR_ERROR0(_("deleting disk is not supported. "
> > - "This may leak data if disk is reassigned"));
> > - ret = 1;
> > - goto cleanup;
> > - } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
> > - /* NB: device not found errors mean the drive was
> > - * auto-deleted and we ignore the error */
> > - ret = 0;
> > - } else {
> > - ret = qemuMonitorJSONCheckError(cmd, reply);
> > - }
> > + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
> > + VIR_DEBUG0(_("drive_del command not found, trying HMP"));
> > + ret = qemuMonitorTextDriveDel(mon, drivestr);
> > + } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
> > + /* NB: device not found errors mean the drive was
> > + * auto-deleted and we ignore the error */
> > + ret = 0;
> > + } else {
> > + ret = qemuMonitorJSONCheckError(cmd, reply);
> > }
>
> Looks good, although I think we should issue the "deleting disk is not
> supported. This may leak data if disk is reassigned" error also in case
> human-monitor-command is not supported. But that will need some more work on
> the HMP infrastructure since it's not possible to get this from
> qemuMonitorText* function that we call.
Is an "unknown command" reply the information we want? If yes this patch
v2 should do the work.
--
Thanks,
Hu Tao
>From 02aae096cd49445b1eb1d0a5e76d100fdde31ebd Mon Sep 17 00:00:00 2001
From: Hu Tao <[email protected]>
Date: Mon, 14 Mar 2011 10:44:47 +0800
Subject: [PATCH v2] qemu: fallback to HMP drive_add/drive_del
fallback to HMP drive_add/drive_del commands if not found in QMP
changes:
v2:
- don't translate debug messages
- report an error to user if HMP drive_del command not found
v1:
- fallback to HMP drive_add/drive_del commands if not found in QMP
---
src/qemu/qemu_monitor_json.c | 38 ++++++++++++++++++++++----------------
1 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index d97dc68..bf2b403 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2359,11 +2359,18 @@ int qemuMonitorJSONAddDrive(qemuMonitorPtr mon,
if (!cmd)
return -1;
- ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+ if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply) < 0))
+ goto cleanup;
- if (ret == 0)
- ret = qemuMonitorJSONCheckError(cmd, reply);
+ if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+ VIR_DEBUG0("drive_add command not found, trying HMP");
+ ret = qemuMonitorTextAddDrive(mon, drivestr);
+ goto cleanup;
+ }
+
+ ret = qemuMonitorJSONCheckError(cmd, reply);
+cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
@@ -2384,22 +2391,21 @@ int qemuMonitorJSONDriveDel(qemuMonitorPtr mon,
if (!cmd)
return -1;
- ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+ if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
+ goto cleanup;
- if (ret == 0) {
- /* See if drive_del isn't supported */
- if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+ if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+ VIR_DEBUG0("drive_del command not found, trying HMP");
+ ret = qemuMonitorTextDriveDel(mon, drivestr);
+ if (ret == 1)
VIR_ERROR0(_("deleting disk is not supported. "
"This may leak data if disk is reassigned"));
- ret = 1;
- goto cleanup;
- } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
- /* NB: device not found errors mean the drive was
- * auto-deleted and we ignore the error */
- ret = 0;
- } else {
- ret = qemuMonitorJSONCheckError(cmd, reply);
- }
+ } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
+ /* NB: device not found errors mean the drive was
+ * auto-deleted and we ignore the error */
+ ret = 0;
+ } else {
+ ret = qemuMonitorJSONCheckError(cmd, reply);
}
cleanup:
--
1.7.3.1
--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list
