8:20 a.m.
This version 2 of this series implements the third alternative (suggested by
Dan Berrange) for falling back to HMP if appropriate snapshot command does not
exist in QMP.
Version 2:
- refactor text monitor commands into parts which can be called directly from
json monitor code
- implement HMP fallback for loadvm and delvm in addition to savevm
Jiri Denemark (4):
qemu: Refactor text monitor snapshot commands
qemu: Fallback to HMP when savevm QMP command is not found
qemu: Refactor qemuDomainSnapshotCreateXML
qemu: Stop guest CPUs before creating a snapshot
src/qemu/qemu_driver.c | 156 ++++++++++++++++++++++----------
src/qemu/qemu_monitor_json.c | 204 +++++++++++++++++++++++++++++++++++-------
src/qemu/qemu_monitor_text.c | 179 ++++++++++++++++++++++++++-----------
src/qemu/qemu_monitor_text.h | 8 ++
4 files changed, 411 insertions(+), 136 deletions(-)
--
1.7.4.1
8:20 a.m.
New subject: [libvirt] [PATCH v2 1/4] qemu: Refactor text monitor snapshot commands
qemuMonitorText{Create,Load,Delete}Snapshot command were refactored so
that parts of the code that build qemu commands and the parts that check
qemu reply can be used separately from JSON monitor code. Two new
functions were created for each of those commands
qemuMonitorText*Snapshot{MakeCommand,CheckReply}
This patch also fixes escaping of {save,load,del}vm argument.
---
src/qemu/qemu_monitor_text.c | 179 ++++++++++++++++++++++++++++++------------
src/qemu/qemu_monitor_text.h | 8 ++
2 files changed, 135 insertions(+), 52 deletions(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 0aed690..0024c5d 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -2568,100 +2568,149 @@ cleanup:
return ret;
}
-int qemuMonitorTextCreateSnapshot(qemuMonitorPtr mon, const char *name)
+
+char *
+qemuMonitorTextCreateSnapshotMakeCommand(const char *name)
{
- char *cmd;
- char *reply = NULL;
- int ret = -1;
+ char *cmd = NULL;
+ char *safename;
- if (virAsprintf(&cmd, "savevm \"%s\"", name) < 0) {
+ if (!(safename = qemuMonitorEscapeArg(name)) ||
+ virAsprintf(&cmd, "savevm \"%s\"", safename) < 0) {
virReportOOMError();
- return -1;
- }
-
- if (qemuMonitorCommand(mon, cmd, &reply)) {
- qemuReportError(VIR_ERR_OPERATION_FAILED,
- _("failed to take snapshot using command
'%s'"), cmd);
goto cleanup;
}
+cleanup:
+ VIR_FREE(safename);
+
+ return cmd;
+}
+
+int
+qemuMonitorTextCreateSnapshotCheckReply(const char *reply)
+{
if (strstr(reply, "Error while creating snapshot") != NULL) {
qemuReportError(VIR_ERR_OPERATION_FAILED,
_("Failed to take snapshot: %s"), reply);
- goto cleanup;
+ return -1;
}
else if (strstr(reply, "No block device can accept snapshots") != NULL) {
qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("this domain does not have a device to take
snapshots"));
- goto cleanup;
+ _("this domain does not have a device"
+ " to take snapshots"));
+ return -1;
}
else if (strstr(reply, "Could not open VM state file") != NULL) {
qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", reply);
- goto cleanup;
+ return -1;
}
else if (strstr(reply, "Error") != NULL
&& strstr(reply, "while writing VM") != NULL) {
qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", reply);
- goto cleanup;
+ return -1;
}
- ret = 0;
-
-cleanup:
- VIR_FREE(cmd);
- VIR_FREE(reply);
- return ret;
+ return 0;
}
-int qemuMonitorTextLoadSnapshot(qemuMonitorPtr mon, const char *name)
+int
+qemuMonitorTextCreateSnapshot(qemuMonitorPtr mon, const char *name)
{
char *cmd;
char *reply = NULL;
int ret = -1;
- if (virAsprintf(&cmd, "loadvm \"%s\"", name) < 0) {
- virReportOOMError();
+ if (!(cmd = qemuMonitorTextCreateSnapshotMakeCommand(name)))
return -1;
- }
if (qemuMonitorCommand(mon, cmd, &reply)) {
qemuReportError(VIR_ERR_OPERATION_FAILED,
- _("failed to restore snapshot using command
'%s'"),
- cmd);
+ _("failed to take snapshot using command
'%s'"), cmd);
+ goto cleanup;
+ }
+
+ ret = qemuMonitorTextCreateSnapshotCheckReply(reply);
+
+cleanup:
+ VIR_FREE(cmd);
+ VIR_FREE(reply);
+ return ret;
+}
+
+
+char *
+qemuMonitorTextLoadSnapshotMakeCommand(const char *name)
+{
+ char *cmd = NULL;
+ char *safename;
+
+ if (!(safename = qemuMonitorEscapeArg(name)) ||
+ virAsprintf(&cmd, "loadvm \"%s\"", safename) < 0) {
+ virReportOOMError();
goto cleanup;
}
+cleanup:
+ VIR_FREE(safename);
+
+ return cmd;
+}
+
+int
+qemuMonitorTextLoadSnapshotCheckReply(const char *reply, const char *name)
+{
if (strstr(reply, "No block device supports snapshots") != NULL) {
qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("this domain does not have a device to load
snapshots"));
- goto cleanup;
+ _("this domain does not have a device"
+ " to load snapshots"));
+ return -1;
}
else if (strstr(reply, "Could not find snapshot") != NULL) {
qemuReportError(VIR_ERR_OPERATION_INVALID,
- _("the snapshot '%s' does not exist, and was not
loaded"),
- name);
- goto cleanup;
+ _("the snapshot '%s' does not exist,"
+ " and was not loaded"), name);
+ return -1;
}
else if (strstr(reply, "Snapshots not supported on device") != NULL) {
qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", reply);
- goto cleanup;
+ return -1;
}
else if (strstr(reply, "Could not open VM state file") != NULL) {
qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", reply);
- goto cleanup;
+ return -1;
}
else if (strstr(reply, "Error") != NULL
&& strstr(reply, "while loading VM state") != NULL) {
qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", reply);
- goto cleanup;
+ return -1;
}
else if (strstr(reply, "Error") != NULL
&& strstr(reply, "while activating snapshot on") != NULL)
{
qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", reply);
+ return -1;
+ }
+
+ return 0;
+}
+
+int qemuMonitorTextLoadSnapshot(qemuMonitorPtr mon, const char *name)
+{
+ char *cmd;
+ char *reply = NULL;
+ int ret = -1;
+
+ if (!(cmd = qemuMonitorTextLoadSnapshotMakeCommand(name)))
+ return -1;
+
+ if (qemuMonitorCommand(mon, cmd, &reply)) {
+ qemuReportError(VIR_ERR_OPERATION_FAILED,
+ _("failed to restore snapshot using command
'%s'"),
+ cmd);
goto cleanup;
}
- ret = 0;
+ ret = qemuMonitorTextLoadSnapshotCheckReply(reply, name);
cleanup:
VIR_FREE(cmd);
@@ -2669,39 +2718,64 @@ cleanup:
return ret;
}
-int qemuMonitorTextDeleteSnapshot(qemuMonitorPtr mon, const char *name)
+
+char *
+qemuMonitorTextDeleteSnapshotMakeCommand(const char *name)
{
- char *cmd;
- char *reply = NULL;
- int ret = -1;
+ char *cmd = NULL;
+ char *safename;
- if (virAsprintf(&cmd, "delvm \"%s\"", name) < 0) {
+ if (!(safename = qemuMonitorEscapeArg(name)) ||
+ virAsprintf(&cmd, "delvm \"%s\"", safename) < 0) {
virReportOOMError();
- return -1;
- }
- if (qemuMonitorCommand(mon, cmd, &reply)) {
- qemuReportError(VIR_ERR_OPERATION_FAILED,
- _("failed to delete snapshot using command
'%s'"),
- cmd);
goto cleanup;
}
+cleanup:
+ VIR_FREE(safename);
+
+ return cmd;
+}
+
+int
+qemuMonitorTextDeleteSnapshotCheckReply(const char *reply)
+{
if (strstr(reply, "No block device supports snapshots") != NULL) {
qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("this domain does not have a device to delete
snapshots"));
- goto cleanup;
+ _("this domain does not have a device"
+ " to delete snapshots"));
+ return 1;
}
else if (strstr(reply, "Snapshots not supported on device") != NULL) {
qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", reply);
- goto cleanup;
+ return 1;
}
else if (strstr(reply, "Error") != NULL
&& strstr(reply, "while deleting snapshot") != NULL) {
qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", reply);
+ return 1;
+ }
+
+ return 0;
+}
+
+int qemuMonitorTextDeleteSnapshot(qemuMonitorPtr mon, const char *name)
+{
+ char *cmd;
+ char *reply = NULL;
+ int ret = -1;
+
+ if (!(cmd = qemuMonitorTextDeleteSnapshotMakeCommand(name)))
+ return -1;
+
+ if (qemuMonitorCommand(mon, cmd, &reply)) {
+ qemuReportError(VIR_ERR_OPERATION_FAILED,
+ _("failed to delete snapshot using command
'%s'"),
+ cmd);
goto cleanup;
}
- ret = 0;
+ ret = qemuMonitorTextDeleteSnapshotCheckReply(reply);
cleanup:
VIR_FREE(cmd);
@@ -2709,6 +2783,7 @@ cleanup:
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 b29dbcc..07b6246 100644
--- a/src/qemu/qemu_monitor_text.h
+++ b/src/qemu/qemu_monitor_text.h
@@ -202,8 +202,16 @@ int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon,
const char *passphrase);
int qemuMonitorTextCreateSnapshot(qemuMonitorPtr mon, const char *name);
+char *qemuMonitorTextCreateSnapshotMakeCommand(const char *name);
+int qemuMonitorTextCreateSnapshotCheckReply(const char *reply);
+
int qemuMonitorTextLoadSnapshot(qemuMonitorPtr mon, const char *name);
+char *qemuMonitorTextLoadSnapshotMakeCommand(const char *name);
+int qemuMonitorTextLoadSnapshotCheckReply(const char *reply, const char *name);
+
int qemuMonitorTextDeleteSnapshot(qemuMonitorPtr mon, const char *name);
+char *qemuMonitorTextDeleteSnapshotMakeCommand(const char *name);
+int qemuMonitorTextDeleteSnapshotCheckReply(const char *reply);
int qemuMonitorTextArbitraryCommand(qemuMonitorPtr mon, const char *cmd,
char **reply);
--
1.7.4.1
8:20 a.m.
New subject: [libvirt] [PATCH v2 2/4] qemu: Fallback to HMP when savevm QMP command is not found
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 QMP equivalents of {save,load,del}vm commands are
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
it's HMP equivalent.
---
src/qemu/qemu_monitor_json.c | 204 +++++++++++++++++++++++++++++++++++-------
1 files changed, 170 insertions(+), 34 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index e6903a1..09fef16 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -31,6 +31,7 @@
#include <string.h>
#include <sys/time.h>
+#include "qemu_monitor_text.h"
#include "qemu_monitor_json.h"
#include "qemu_command.h"
#include "memory.h"
@@ -675,6 +676,55 @@ static void qemuMonitorJSONHandleVNCDisconnect(qemuMonitorPtr mon,
virJSONValueP
}
+static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3)
+qemuMonitorJSONHumanCommand(qemuMonitorPtr mon,
+ char **reply_str,
+ const char *cmd_str)
+{
+ virJSONValuePtr cmd = NULL;
+ virJSONValuePtr reply = NULL;
+ virJSONValuePtr obj;
+ int ret = -1;
+
+ cmd = qemuMonitorJSONMakeCommand("human-monitor-command",
+ "s:command-line", cmd_str,
+ NULL);
+
+ if (!cmd || qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ goto cleanup;
+
+ if (qemuMonitorJSONCheckError(cmd, reply))
+ goto cleanup;
+
+ if (!(obj = virJSONValueObjectGet(reply, "return"))) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("human monitor command was missing return data"));
+ goto cleanup;
+ }
+
+ if (reply_str) {
+ const char *data;
+
+ if ((data = virJSONValueGetString(obj)))
+ *reply_str = strdup(data);
+ else
+ *reply_str = strdup("");
+
+ if (!*reply_str) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+
+cleanup:
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+ return ret;
+}
+
+
int
qemuMonitorJSONSetCapabilities(qemuMonitorPtr mon)
{
@@ -2389,6 +2439,34 @@ int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon,
return ret;
}
+
+static int
+qemuMonitorJSONCreateSnapshotHMP(qemuMonitorPtr mon, const char *name)
+{
+ char *cmd;
+ char *reply = NULL;
+ int ret = -1;
+
+ if (!(cmd = qemuMonitorTextCreateSnapshotMakeCommand(name)))
+ return -1;
+
+ if (qemuMonitorJSONHumanCommand(mon, &reply, cmd) < 0) {
+ qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("failed to take snapshot using HMP command"));
+ goto cleanup;
+ }
+
+ if (qemuMonitorTextCreateSnapshotCheckReply(reply) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(cmd);
+ VIR_FREE(reply);
+ return ret;
+}
+
int qemuMonitorJSONCreateSnapshot(qemuMonitorPtr mon, const char *name)
{
int ret;
@@ -2401,16 +2479,51 @@ int qemuMonitorJSONCreateSnapshot(qemuMonitorPtr mon, const char
*name)
if (!cmd)
return -1;
- ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+ if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
+ goto cleanup;
- if (ret == 0)
- ret = qemuMonitorJSONCheckError(cmd, reply);
+ if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+ VIR_DEBUG0("savevm command not found, trying HMP");
+ ret = qemuMonitorJSONCreateSnapshotHMP(mon, name);
+ goto cleanup;
+ }
+
+ ret = qemuMonitorJSONCheckError(cmd, reply);
+cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
}
+
+static int
+qemuMonitorJSONLoadSnapshotHMP(qemuMonitorPtr mon, const char *name)
+{
+ char *cmd;
+ char *reply = NULL;
+ int ret = -1;
+
+ if (!(cmd = qemuMonitorTextLoadSnapshotMakeCommand(name)))
+ return -1;
+
+ if (qemuMonitorJSONHumanCommand(mon, &reply, cmd) < 0) {
+ qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("failed to load snapshot using HMP command"));
+ goto cleanup;
+ }
+
+ if (qemuMonitorTextLoadSnapshotCheckReply(reply, name) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(cmd);
+ VIR_FREE(reply);
+ return ret;
+}
+
int qemuMonitorJSONLoadSnapshot(qemuMonitorPtr mon, const char *name)
{
int ret;
@@ -2423,16 +2536,51 @@ int qemuMonitorJSONLoadSnapshot(qemuMonitorPtr mon, const char
*name)
if (!cmd)
return -1;
- ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+ if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
+ goto cleanup;
- if (ret == 0)
- ret = qemuMonitorJSONCheckError(cmd, reply);
+ if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+ VIR_DEBUG0("loadvm command not found, trying HMP");
+ ret = qemuMonitorJSONLoadSnapshotHMP(mon, name);
+ goto cleanup;
+ }
+
+ ret = qemuMonitorJSONCheckError(cmd, reply);
+cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
}
+
+static int
+qemuMonitorJSONDeleteSnapshotHMP(qemuMonitorPtr mon, const char *name)
+{
+ char *cmd;
+ char *reply = NULL;
+ int ret = -1;
+
+ if (!(cmd = qemuMonitorTextDeleteSnapshotMakeCommand(name)))
+ return -1;
+
+ if (qemuMonitorJSONHumanCommand(mon, &reply, cmd) < 0) {
+ qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("failed to delete snapshot using HMP command"));
+ goto cleanup;
+ }
+
+ if (qemuMonitorTextDeleteSnapshotCheckReply(reply) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(cmd);
+ VIR_FREE(reply);
+ return ret;
+}
+
int qemuMonitorJSONDeleteSnapshot(qemuMonitorPtr mon, const char *name)
{
int ret;
@@ -2445,11 +2593,18 @@ int qemuMonitorJSONDeleteSnapshot(qemuMonitorPtr mon, const char
*name)
if (!cmd)
return -1;
- ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+ if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
+ goto cleanup;
- if (ret == 0)
- ret = qemuMonitorJSONCheckError(cmd, reply);
+ if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+ VIR_DEBUG0("loadvm command not found, trying HMP");
+ ret = qemuMonitorJSONDeleteSnapshotHMP(mon, name);
+ goto cleanup;
+ }
+
+ ret = qemuMonitorJSONCheckError(cmd, reply);
+cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
@@ -2464,36 +2619,17 @@ int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon,
virJSONValuePtr reply = NULL;
int ret = -1;
- if (!hmp) {
- cmd = virJSONValueFromString(cmd_str);
+ if (hmp) {
+ return qemuMonitorJSONHumanCommand(mon, reply_str, cmd_str);
} else {
- cmd = qemuMonitorJSONMakeCommand("human-monitor-command",
- "s:command-line", cmd_str,
- NULL);
- }
-
- if (!cmd)
- return -1;
+ if (!(cmd = virJSONValueFromString(cmd_str)))
+ goto cleanup;
- if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
- goto cleanup;
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ goto cleanup;
- if (!hmp) {
if (!(*reply_str = virJSONValueToString(reply)))
goto cleanup;
- } else if (qemuMonitorJSONCheckError(cmd, reply)) {
- goto cleanup;
- } else {
- const char *data;
- if (!(data = virJSONValueObjectGetString(reply, "return"))) {
- qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("human monitor command was missing return
data"));
- goto cleanup;
- }
- if (!(*reply_str = strdup(data))) {
- virReportOOMError();
- goto cleanup;
- }
}
ret = 0;
--
1.7.4.1
8:20 a.m.
New subject: [libvirt] [PATCH v2 3/4] qemu: Refactor qemuDomainSnapshotCreateXML
---
src/qemu/qemu_driver.c | 125 +++++++++++++++++++++++++++++------------------
1 files changed, 77 insertions(+), 48 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0f7cbad..2b286bb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5990,6 +5990,81 @@ static int qemuDomainSnapshotIsAllowed(virDomainObjPtr vm)
return 1;
}
+/* The domain is expected to be locked and inactive. */
+static int
+qemuDomainSnapshotCreateInactive(virDomainObjPtr vm,
+ virDomainSnapshotObjPtr snap)
+{
+ const char *qemuimgarg[] = { NULL, "snapshot", "-c", NULL, NULL,
NULL };
+ int ret = -1;
+ int i;
+
+ qemuimgarg[0] = qemuFindQemuImgBinary();
+ if (qemuimgarg[0] == NULL) {
+ /* qemuFindQemuImgBinary set the error */
+ goto cleanup;
+ }
+
+ qemuimgarg[3] = snap->def->name;
+
+ for (i = 0; i < vm->def->ndisks; i++) {
+ /* FIXME: we also need to handle LVM here */
+ /* FIXME: if we fail halfway through this loop, we are in an
+ * inconsistent state. I'm not quite sure what to do about that
+ */
+ if (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
+ if (!vm->def->disks[i]->driverType ||
+ STRNEQ(vm->def->disks[i]->driverType, "qcow2")) {
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
+ _("Disk device '%s' does not support"
+ " snapshotting"),
+ vm->def->disks[i]->info.alias);
+ goto cleanup;
+ }
+
+ qemuimgarg[4] = vm->def->disks[i]->src;
+
+ if (virRun(qemuimgarg, NULL) < 0) {
+ virReportSystemError(errno,
+ _("Failed to run '%s' to create
snapshot"
+ " '%s' from disk
'%s'"),
+ qemuimgarg[0], snap->def->name,
+ vm->def->disks[i]->src);
+ goto cleanup;
+ }
+ }
+ }
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(qemuimgarg[0]);
+ return ret;
+}
+
+/* The domain is expected to be locked and active. */
+static int
+qemuDomainSnapshotCreateActive(struct qemud_driver *driver,
+ virDomainObjPtr *vmptr,
+ virDomainSnapshotObjPtr snap)
+{
+ virDomainObjPtr vm = *vmptr;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ int ret;
+
+ if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
+ return -1;
+
+ qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name);
+ qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+ if (qemuDomainObjEndJob(vm) == 0)
+ *vmptr = NULL;
+
+ return ret;
+}
+
static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
const char *xmlDesc,
unsigned int flags)
@@ -6000,8 +6075,6 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr
domain,
virDomainSnapshotPtr snapshot = NULL;
char uuidstr[VIR_UUID_STRING_BUFLEN];
virDomainSnapshotDefPtr def;
- const char *qemuimgarg[] = { NULL, "snapshot", "-c", NULL, NULL,
NULL };
- int i;
virCheckFlags(0, NULL);
@@ -6032,54 +6105,11 @@ static virDomainSnapshotPtr
qemuDomainSnapshotCreateXML(virDomainPtr domain,
/* actually do the snapshot */
if (!virDomainObjIsActive(vm)) {
- qemuimgarg[0] = qemuFindQemuImgBinary();
- if (qemuimgarg[0] == NULL)
- /* qemuFindQemuImgBinary set the error */
+ if (qemuDomainSnapshotCreateInactive(vm, snap) < 0)
goto cleanup;
-
- qemuimgarg[3] = snap->def->name;
-
- for (i = 0; i < vm->def->ndisks; i++) {
- /* FIXME: we also need to handle LVM here */
- /* FIXME: if we fail halfway through this loop, we are in an
- * inconsistent state. I'm not quite sure what to do about that
- */
- if (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
- if (!vm->def->disks[i]->driverType ||
- STRNEQ(vm->def->disks[i]->driverType, "qcow2")) {
- qemuReportError(VIR_ERR_OPERATION_INVALID,
- _("Disk device '%s' does not support
snapshotting"),
- vm->def->disks[i]->info.alias);
- goto cleanup;
- }
-
- qemuimgarg[4] = vm->def->disks[i]->src;
-
- if (virRun(qemuimgarg, NULL) < 0) {
- virReportSystemError(errno,
- _("Failed to run '%s' to create
snapshot '%s' from disk '%s'"),
- qemuimgarg[0], snap->def->name,
- vm->def->disks[i]->src);
- goto cleanup;
- }
- }
- }
}
else {
- qemuDomainObjPrivatePtr priv;
- int ret;
-
- if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
- goto cleanup;
- priv = vm->privateData;
- qemuDomainObjEnterMonitorWithDriver(driver, vm);
- ret = qemuMonitorCreateSnapshot(priv->mon, def->name);
- qemuDomainObjExitMonitorWithDriver(driver, vm);
- if (qemuDomainObjEndJob(vm) == 0) {
- vm = NULL;
- goto cleanup;
- }
- if (ret < 0)
+ if (qemuDomainSnapshotCreateActive(driver, &vm, snap) < 0)
goto cleanup;
}
@@ -6109,7 +6139,6 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr
domain,
snapshot = virGetDomainSnapshot(domain, snap->def->name);
cleanup:
- VIR_FREE(qemuimgarg[0]);
if (vm)
virDomainObjUnlock(vm);
qemuDriverUnlock(driver);
--
1.7.4.1
8:20 a.m.
New subject: [libvirt] [PATCH v2 4/4] qemu: Stop guest CPUs before creating a snapshot
This is currently needed because libvirt doesn't support qemu's RESUME
event. I have already implemented it but I need to test it thoroughly
since it has a great potential to break things when libvirt starts
seeing resume events that it wasn't seeing before.
---
src/qemu/qemu_driver.c | 37 ++++++++++++++++++++++++++++++++-----
1 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2b286bb..d5776d6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6044,21 +6044,47 @@ cleanup:
/* The domain is expected to be locked and active. */
static int
-qemuDomainSnapshotCreateActive(struct qemud_driver *driver,
+qemuDomainSnapshotCreateActive(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr *vmptr,
virDomainSnapshotObjPtr snap)
{
virDomainObjPtr vm = *vmptr;
qemuDomainObjPrivatePtr priv = vm->privateData;
- int ret;
+ bool resume = false;
+ int ret = -1;
if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
return -1;
+ if (vm->state == VIR_DOMAIN_RUNNING) {
+ /* savevm monitor command pauses the domain emitting an event which
+ * confuses libvirt since it's not notified when qemu resumes the
+ * domain. Thus we stop and start CPUs ourselves.
+ */
+ if (qemuProcessStopCPUs(driver, vm) < 0)
+ goto cleanup;
+
+ resume = true;
+ if (!virDomainObjIsActive(vm)) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("guest unexpectedly quit"));
+ goto cleanup;
+ }
+ }
+
qemuDomainObjEnterMonitorWithDriver(driver, vm);
ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name);
qemuDomainObjExitMonitorWithDriver(driver, vm);
+cleanup:
+ if (resume && virDomainObjIsActive(vm) &&
+ qemuProcessStartCPUs(driver, vm, conn) < 0 &&
+ virGetLastError() == NULL) {
+ qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("resuming after snapshot failed"));
+ }
+
if (qemuDomainObjEndJob(vm) == 0)
*vmptr = NULL;
@@ -6103,18 +6129,19 @@ static virDomainSnapshotPtr
qemuDomainSnapshotCreateXML(virDomainPtr domain,
if (!(snap = virDomainSnapshotAssignDef(&vm->snapshots, def)))
goto cleanup;
+ snap->def->state = vm->state;
+
/* actually do the snapshot */
if (!virDomainObjIsActive(vm)) {
if (qemuDomainSnapshotCreateInactive(vm, snap) < 0)
goto cleanup;
}
else {
- if (qemuDomainSnapshotCreateActive(driver, &vm, snap) < 0)
+ if (qemuDomainSnapshotCreateActive(domain->conn, driver,
+ &vm, snap) < 0)
goto cleanup;
}
- snap->def->state = vm->state;
-
/* FIXME: if we fail after this point, there's not a whole lot we can
* do; we've successfully taken the snapshot, and we are now running
* on it, so we have to go forward the best we can
--
1.7.4.1
5038
days inactive
5038
days old
4 comments
1 participants
participants (1)
-
Jiri Denemark