Re: [libvirt] [PATCH] 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 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;