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

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



On 2012年12月04日 05:33, Eric Blake wrote:
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.

ACK'ed a bit earlier, there is no testing for this new flag,
and...

---
  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;

"ret" can be used without initialization.


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