[libvirt] [PATCH] qemu: nicer error message if live disk snapshot unsupported

Eric Blake eblake at redhat.com
Mon Dec 3 21:33:27 UTC 2012


Without this patch, attempts to create a disk snapshot when qemu
is too old results in a cryptic message:

virsh # snapshot-create 23 --disk-only
error: operation failed: Failed to take snapshot: unknown command: 'snapshot_blkdev'

Now it reports:

virsh # snapshot-create 23 --disk-only
error: unsupported configuration: live disk snapshot not supported with this QEMU binary

All versions of qemu that support live disk snapshot also support
QMP (basically upstream qemu 1.1 and later, and backport to RHEL 6.2).

* src/qemu/qemu_capabilities.h (QEMU_CAPS_DISK_SNAPSHOT): New
capability.
* src/qemu/qemu_capabilities.c (qemuCaps): Track it.
(qemuCapsProbeQMPCommands): Set it.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateDiskActive): Use
it.
* src/qemu/qemu_monitor.c (qemuMonitorDiskSnapshot): Simplify.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONDiskSnapshot):
Likewise.
* src/qemu/qemu_monitor_text.h (qemuMonitorTextDiskSnapshot):
Delete.
* src/qemu/qemu_monitor_text.c (qemuMonitorTextDiskSnapshot):
Likewise.
---
 src/qemu/qemu_capabilities.c |    3 +++
 src/qemu/qemu_capabilities.h |    1 +
 src/qemu/qemu_driver.c       |    5 +++++
 src/qemu/qemu_monitor.c      |   13 ++++---------
 src/qemu/qemu_monitor_json.c |    6 ------
 src/qemu/qemu_monitor_text.c |   37 -------------------------------------
 src/qemu/qemu_monitor_text.h |    4 ----
 7 files changed, 13 insertions(+), 56 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 6e34cdf..668935e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -193,6 +193,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
               "drive-mirror", /* 115 */
               "usb-redir.bootindex",
               "usb-host.bootindex",
+              "blockdev-snapshot-sync",
     );

 struct _qemuCaps {
@@ -1948,6 +1949,8 @@ qemuCapsProbeQMPCommands(qemuCapsPtr caps,
             qemuCapsSet(caps, QEMU_CAPS_VNC);
         else if (STREQ(name, "drive-mirror"))
             qemuCapsSet(caps, QEMU_CAPS_DRIVE_MIRROR);
+        else if (STREQ(name, "blockdev-snapshot-sync"))
+            qemuCapsSet(caps, QEMU_CAPS_DISK_SNAPSHOT);
         VIR_FREE(name);
     }
     VIR_FREE(commands);
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index f420c43..3da8672 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -155,6 +155,7 @@ enum qemuCapsFlags {
     QEMU_CAPS_DRIVE_MIRROR       = 115, /* drive-mirror monitor command */
     QEMU_CAPS_USB_REDIR_BOOTINDEX = 116, /* usb-redir.bootindex */
     QEMU_CAPS_USB_HOST_BOOTINDEX = 117, /* usb-host.bootindex */
+    QEMU_CAPS_DISK_SNAPSHOT      = 118, /* blockdev-snapshot-sync command */

     QEMU_CAPS_LAST,                   /* this must always be the last item */
 };
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8e838cd..fbacd8b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11230,6 +11230,11 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
             virReportOOMError();
             goto cleanup;
         }
+    } else if (!qemuCapsGet(priv->caps, QEMU_CAPS_DISK_SNAPSHOT)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("live disk snapshot not supported with this "
+                         "QEMU binary"));
+        goto cleanup;
     }

     /* No way to roll back if first disk succeeds but later disks
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index f85bb76..543f714 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2769,17 +2769,12 @@ qemuMonitorDiskSnapshot(qemuMonitorPtr mon, virJSONValuePtr actions,
         return -1;
     }

-    if (mon->json) {
+    if (mon->json)
         ret = qemuMonitorJSONDiskSnapshot(mon, actions, device, file, format,
                                           reuse);
-    } else {
-        if (actions || STRNEQ(format, "qcow2") || reuse) {
-            virReportError(VIR_ERR_INVALID_ARG, "%s",
-                           _("text monitor lacks several snapshot features"));
-            return -1;
-        }
-        ret = qemuMonitorTextDiskSnapshot(mon, device, file);
-    }
+    else
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("disk snapshot requires JSON monitor"));
     return ret;
 }

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 9b6702a..0cd66b6 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2986,12 +2986,6 @@ qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon, virJSONValuePtr actions,
         if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
             goto cleanup;

-        if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
-            VIR_DEBUG("blockdev-snapshot-sync command not found, trying HMP");
-            ret = qemuMonitorTextDiskSnapshot(mon, device, file);
-            goto cleanup;
-        }
-
         ret = qemuMonitorJSONCheckError(cmd, reply);
     }

diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 43e5449..fa10600 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -2958,43 +2958,6 @@ cleanup:
     return ret;
 }

-int
-qemuMonitorTextDiskSnapshot(qemuMonitorPtr mon, const char *device,
-                            const char *file)
-{
-    char *cmd = NULL;
-    char *reply = NULL;
-    int ret = -1;
-    char *safename;
-
-    if (!(safename = qemuMonitorEscapeArg(file)) ||
-        virAsprintf(&cmd, "snapshot_blkdev %s \"%s\"", device, safename) < 0) {
-        virReportOOMError();
-        goto cleanup;
-    }
-
-    if (qemuMonitorHMPCommand(mon, cmd, &reply))
-        goto cleanup;
-
-    if (strstr(reply, "error while creating qcow2") != NULL ||
-        strstr(reply, "unknown command:") != NULL) {
-        virReportError(VIR_ERR_OPERATION_FAILED,
-                       _("Failed to take snapshot: %s"), reply);
-        goto cleanup;
-    }
-
-    /* XXX Should we scrape 'info block' output for
-     * 'device:... file=name backing_file=oldname' to make sure the
-     * command succeeded?  */
-
-    ret = 0;
-
-cleanup:
-    VIR_FREE(safename);
-    VIR_FREE(cmd);
-    VIR_FREE(reply);
-    return ret;
-}

 int qemuMonitorTextArbitraryCommand(qemuMonitorPtr mon, const char *cmd,
                                     char **reply)
diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h
index 079fbdc..97abad6 100644
--- a/src/qemu/qemu_monitor_text.h
+++ b/src/qemu/qemu_monitor_text.h
@@ -217,10 +217,6 @@ int qemuMonitorTextCreateSnapshot(qemuMonitorPtr mon, const char *name);
 int qemuMonitorTextLoadSnapshot(qemuMonitorPtr mon, const char *name);
 int qemuMonitorTextDeleteSnapshot(qemuMonitorPtr mon, const char *name);

-int qemuMonitorTextDiskSnapshot(qemuMonitorPtr mon,
-                                const char *device,
-                                const char *file);
-
 int qemuMonitorTextArbitraryCommand(qemuMonitorPtr mon, const char *cmd,
                                     char **reply);

-- 
1.7.1




More information about the libvir-list mailing list