qemu: nicer error message if live disk snapshot unsupported

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 backports 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.

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 6e34cdf93a02e9c246b9b06919f6d98304e09122..668935e913d274afe95218a2e11246f617d1d9bb 100644 (file)
--- 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 f420c4385a2b20ad7e9f0aa2416942cf3b9bf169..3da86725469f6c9dfa8fabb34cfebf2d1c4c65e4 100644 (file)
--- 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 bb1ec059a2ab01fc75307b68261cea5d1e7f93e1..a0503148f0573e477a9bbba10836292e20ebf409 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11281,6 +11281,11 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
             virReportOOMError();
             goto cleanup;
         }
+    } else if (!qemuCapsGet(priv->caps, QEMU_CAPS_DISK_SNAPSHOT)) {
+        virReportError(VIR_ERR_OPERATION_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 f85bb76c6b659647dd3fb8c3b17e2eb2209c2c7c..066e420eb9354a501e2d984555284a1e715693fc 100644 (file)
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2758,7 +2758,7 @@ qemuMonitorDiskSnapshot(qemuMonitorPtr mon, virJSONValuePtr actions,
                         const char *device, const char *file,
                         const char *format, bool reuse)
 {
-    int ret;
+    int ret = -1;
 
     VIR_DEBUG("mon=%p, actions=%p, device=%s, file=%s, format=%s, reuse=%d",
               mon, actions, device, file, format, reuse);
@@ -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 9b6702a0b11cb6f9d0434e9fdb2ecc3c8206cf71..0cd66b694e8df870e8d7b1763518c8d033a30792 100644 (file)
--- 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 43e5449cec038377b510795a87e92888c026e9b8..fa106005de9d6a73fe94aa7ef7b4cf54d3dd490a 100644 (file)
--- 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 079fbdce39c5f0324f2a695e8d0fd40d9d11dd40..97abad660dc8ee4c3d433ab819c586227c93e1e3 100644 (file)
--- 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);