[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH v2] qemu: fallback to HMP drive_add/drive_del



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 <hutao cn fujitsu com>
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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]