On Fri, Feb 25, 2011 at 07:04:08PM +0100, Jiri Denemark wrote:
qemu driver in libvirt gained support for creating domain snapshots almost a year ago in libvirt 0.8.0. Since then we enabled QMP support for qemu >= 0.13.0 but a QMP equivalent of savevm command is not implemented in current qemu (0.14.0) so the domain snapshot support is not very useful.
This patch detects when the appropriate QMP command is not implemented and tries to use human-monitor-command (aka HMP passthrough) to run savevm HMP command. --- src/qemu/qemu_monitor_json.c | 144 +++++++++++++++++++++++++++++++++-------- 1 files changed, 116 insertions(+), 28 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e6903a1..6dd7980 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -675,6 +675,65 @@ static void qemuMonitorJSONHandleVNCDisconnect(qemuMonitorPtr mon, virJSONValueP }
+static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_FMT_PRINTF(3, 4) +qemuMonitorJSONHumanCommand(qemuMonitorPtr mon, + char **reply_str, + const char *fmt, ...) +{ + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + virJSONValuePtr obj; + char *cmd_str = NULL; + va_list ap; + int ret = -1; + + va_start(ap, fmt); + if (virVasprintf(&cmd_str, fmt, ap) < 0) { + virReportOOMError(); + goto cleanup; + }
This is a little dodgy because it doesn't take care of any data escaping of special characters in the args.
@@ -2389,6 +2448,47 @@ int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, return ret; }
+ +static int +qemuMonitorJSONCreateSnapshotHMP(qemuMonitorPtr mon, const char *name) +{ + char *reply = NULL; + int ret = -1; + + if (qemuMonitorJSONHumanCommand(mon, &reply, "savevm \"%s\"", name) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("failed to take snapshot using HMP command")); + goto cleanup; + } + + if (strstr(reply, "Error while creating snapshot")) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("failed to take snapshot: %s"), reply); + goto cleanup; + } + else if (strstr(reply, "No block device can accept snapshots")) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("this domain does not have a device to" + " take snapshots")); + goto cleanup; + } + else if (strstr(reply, "Could not open VM state file")) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", reply); + goto cleanup; + } + else if (strstr(reply, "Error") + && strstr(reply, "while writing VM")) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", reply); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(reply); + return ret; +}
Could we re-factor the qemuMonitorTextCreateSnapshot() command so that the code which generates the command string & the code which checks the reply are separate callback methods. Then, this JSON code could do something like static int qemuMonitorJSONCreateSnapshotHMP(qemuMonitorPtr mon, const char *name) { if (!(cmdstr = qemuMonitorTextMakeCreateSnapshotCommand(...))) return -1; reply = qemuMonitorJSONHumanCommand(cmdstr) VIR_FREE(cmdstr); if (!reply) return -1; if (qemuMonitorTextCheckCreateSnapshotReply(reply) < 0) return -1; return 0; } Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|