[libvirt] [PATCH 0/3] Use guest agent to quiesce disks
Since we have qemu guest agent support in libvirt, we can start wiring up some things that GA already knows how to do. One of them is file system freeze and thaw. Domain snapshots can profit from this functionality. Michal Privoznik (3): qemu_agent: Create file system freeze and thaw functions snapshots: Introduce VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag virsh: Expose new VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag include/libvirt/libvirt.h.in | 4 ++ src/libvirt.c | 6 ++ src/qemu/qemu_agent.c | 68 ++++++++++++++++++++++++ src/qemu/qemu_agent.h | 3 + src/qemu/qemu_driver.c | 118 ++++++++++++++++++++++++++++++++++++----- tools/virsh.c | 6 ++ tools/virsh.pod | 14 ++++- 7 files changed, 202 insertions(+), 17 deletions(-) -- 1.7.3.4
These functions simply issue command to guest agent which should freeze or unfreeze all file systems within guest. --- src/qemu/qemu_agent.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 3 ++ 2 files changed, 71 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 6a7c7b3..3a5cc4b 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1110,3 +1110,71 @@ int qemuAgentShutdown(qemuAgentPtr mon, virJSONValueFree(reply); return ret; } + +/* + * qemuAgentFSFreeze: + * @mon: Agent + * + * Issue guest-fsfreeze-freeze command to guest agent, + * which freezes all mounted file systems and returns + * number of frozen file systems on success. + * + * Returns: number of file system frozen on success, + * -1 on error. + */ +int qemuAgentFSFreeze(qemuAgentPtr mon) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = qemuAgentMakeCommand("guest-fsfreeze-freeze", NULL); + + if (!cmd) + return -1; + + if (qemuAgentCommand(mon, cmd, &reply) < 0 || + qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + virJSONValueObjectGetNumberInt(reply, "return", &ret); + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + +/* + * qemuAgentFSThaw: + * @mon: Agent + * + * Issue guest-fsfreeze-thaw command to guest agent, + * which unfreezes all mounted file systems and returns + * number of thawed file systems on success. + * + * Returns: number of file system thawed on success, + * -1 on error. + */ +int qemuAgentFSThaw(qemuAgentPtr mon) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = qemuAgentMakeCommand("guest-fsfreeze-thaw", NULL); + + if (!cmd) + return -1; + + if (qemuAgentCommand(mon, cmd, &reply) < 0 || + qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + virJSONValueObjectGetNumberInt(reply, "return", &ret); + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 93c2ae7..df59ef7 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -66,4 +66,7 @@ typedef enum { int qemuAgentShutdown(qemuAgentPtr mon, qemuAgentShutdownMode mode); +int qemuAgentFSFreeze(qemuAgentPtr mon); +int qemuAgentFSThaw(qemuAgentPtr mon); + #endif /* __QEMU_AGENT_H__ */ -- 1.7.3.4
On Tue, Jan 24, 2012 at 18:37:43 +0100, Michal Privoznik wrote:
These functions simply issue command to guest agent which should freeze or unfreeze all file systems within guest. --- src/qemu/qemu_agent.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 3 ++ 2 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 6a7c7b3..3a5cc4b 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1110,3 +1110,71 @@ int qemuAgentShutdown(qemuAgentPtr mon, virJSONValueFree(reply); return ret; } + +/* + * qemuAgentFSFreeze: + * @mon: Agent + * + * Issue guest-fsfreeze-freeze command to guest agent, + * which freezes all mounted file systems and returns + * number of frozen file systems on success. + * + * Returns: number of file system frozen on success, + * -1 on error. + */ +int qemuAgentFSFreeze(qemuAgentPtr mon) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = qemuAgentMakeCommand("guest-fsfreeze-freeze", NULL); + + if (!cmd) + return -1; + + if (qemuAgentCommand(mon, cmd, &reply) < 0 || + qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + virJSONValueObjectGetNumberInt(reply, "return", &ret);
Doesn't the above function need to be checked for errors? That is, what if there's no "return" key in the reply? In json monitor code, we usually have something like the following code: if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("guest-fsfreeze-freeze reply was missing return" " data")); }
+ +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + +/* + * qemuAgentFSThaw: + * @mon: Agent + * + * Issue guest-fsfreeze-thaw command to guest agent, + * which unfreezes all mounted file systems and returns + * number of thawed file systems on success. + * + * Returns: number of file system thawed on success, + * -1 on error. + */ +int qemuAgentFSThaw(qemuAgentPtr mon) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = qemuAgentMakeCommand("guest-fsfreeze-thaw", NULL); + + if (!cmd) + return -1; + + if (qemuAgentCommand(mon, cmd, &reply) < 0 || + qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + virJSONValueObjectGetNumberInt(reply, "return", &ret);
And similar check for this line as well.
+ +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 93c2ae7..df59ef7 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -66,4 +66,7 @@ typedef enum { int qemuAgentShutdown(qemuAgentPtr mon, qemuAgentShutdownMode mode);
+int qemuAgentFSFreeze(qemuAgentPtr mon); +int qemuAgentFSThaw(qemuAgentPtr mon); + #endif /* __QEMU_AGENT_H__ */ -- 1.7.3.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 24.01.2012 19:21, Jiri Denemark wrote:
On Tue, Jan 24, 2012 at 18:37:43 +0100, Michal Privoznik wrote:
+ + if (qemuAgentCommand(mon, cmd, &reply) < 0 || + qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + virJSONValueObjectGetNumberInt(reply, "return", &ret);
Doesn't the above function need to be checked for errors? That is, what if there's no "return" key in the reply? In json monitor code, we usually have something like the following code:
if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("guest-fsfreeze-freeze reply was missing return" " data")); }
In fact this check is done in qemuAgentCheckError(); which reports error as well. But I agree I should check if returned value is integer and throw an error. I have squashed this into my local branch: diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 3a5cc4b..9df5546 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1137,7 +1137,10 @@ int qemuAgentFSFreeze(qemuAgentPtr mon) qemuAgentCheckError(cmd, reply) < 0) goto cleanup; - virJSONValueObjectGetNumberInt(reply, "return", &ret); + if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed return value")); + } cleanup: virJSONValueFree(cmd); @@ -1171,7 +1174,10 @@ int qemuAgentFSThaw(qemuAgentPtr mon) qemuAgentCheckError(cmd, reply) < 0) goto cleanup; - virJSONValueObjectGetNumberInt(reply, "return", &ret); + if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed return value")); + } cleanup: virJSONValueFree(cmd);
With this flag, virDomainSnapshotCreate will use fs-freeze and fs-thaw guest agent commands to quiesce guest's disks. --- include/libvirt/libvirt.h.in | 4 ++ src/libvirt.c | 6 ++ src/qemu/qemu_driver.c | 118 ++++++++++++++++++++++++++++++++++++----- 3 files changed, 113 insertions(+), 15 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5e6e488..182065d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3125,6 +3125,10 @@ typedef enum { system checkpoint */ VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT = (1 << 5), /* reuse any existing external files */ + VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE = (1 << 6), /* use guest agent to + quiesce all mounted + file systems within + the domain */ } virDomainSnapshotCreateFlags; /* Take a snapshot of the current VM state */ diff --git a/src/libvirt.c b/src/libvirt.c index 96ad3d5..d9b8d88 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16602,6 +16602,12 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot) * inconsistent (as if power had been pulled), and specifying this * with the VIR_DOMAIN_SNAPSHOT_CREATE_HALT flag risks data loss. * + * If @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE, then the + * libvirt will attempt to use guest agent to freeze and thaw all + * file systems in use within domain OS. However, if the guest agent + * is not present, an error is trowed. Moreover, this flags requires + * VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY to be passed as well. + * * By default, if the snapshot involves external files, and any of the * destination files already exist as a regular file, the snapshot is * rejected to avoid losing contents of those files. However, if diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c81289a..14ad30b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9461,6 +9461,56 @@ static int qemuDomainSnapshotIsAllowed(virDomainObjPtr vm) return 1; } +static int +qemuDomainSnapshotFSFreeze(struct qemud_driver *driver, + virDomainObjPtr vm) { + qemuDomainObjPrivatePtr priv = vm->privateData; + int freezed; + + if (priv->agentError) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QEMU guest agent is not " + "available due to an error")); + return -1; + } + if (!priv->agent) { + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + return -1; + } + + qemuDomainObjEnterAgent(driver, vm); + freezed = qemuAgentFSFreeze(priv->agent); + qemuDomainObjExitAgent(driver, vm); + + return freezed; +} + +static int +qemuDomainSnapshotFSThaw(struct qemud_driver *driver, + virDomainObjPtr vm) { + qemuDomainObjPrivatePtr priv = vm->privateData; + int thawed; + + if (priv->agentError) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QEMU guest agent is not " + "available due to an error")); + return -1; + } + if (!priv->agent) { + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + return -1; + } + + qemuDomainObjEnterAgent(driver, vm); + thawed = qemuAgentFSThaw(priv->agent); + qemuDomainObjExitAgent(driver, vm); + + return thawed; +} + /* The domain is expected to be locked and inactive. */ static int qemuDomainSnapshotCreateInactive(struct qemud_driver *driver, @@ -9493,6 +9543,13 @@ qemuDomainSnapshotCreateActive(virConnectPtr conn, } if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) && + (qemuDomainSnapshotFSFreeze(driver, vm) < 0)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to freeze domain's file systems")); + goto endjob; + } + /* 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. @@ -9532,13 +9589,21 @@ qemuDomainSnapshotCreateActive(virConnectPtr conn, } cleanup: - if (resume && virDomainObjIsActive(vm) && - qemuProcessStartCPUs(driver, vm, conn, - VIR_DOMAIN_RUNNING_UNPAUSED, - QEMU_ASYNC_JOB_NONE) < 0 && - virGetLastError() == NULL) { - qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("resuming after snapshot failed")); + if (resume && virDomainObjIsActive(vm)) { + if (qemuProcessStartCPUs(driver, vm, conn, + VIR_DOMAIN_RUNNING_UNPAUSED, + QEMU_ASYNC_JOB_NONE) < 0 && + virGetLastError() == NULL) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("resuming after snapshot failed")); + goto endjob; + } + + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) && + qemuDomainSnapshotFSThaw(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to thaw domain's file systems")); + } } endjob: @@ -9773,6 +9838,13 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn, if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) && + (qemuDomainSnapshotFSFreeze(driver, vm) < 0)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to freeze domain's file systems")); + goto endjob; + } + /* In qemu, snapshot_blkdev on a single disk will pause cpus, * but this confuses libvirt since notifications are not given * when qemu resumes. And for multiple disks, libvirt must @@ -9840,13 +9912,21 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn, } cleanup: - if (resume && virDomainObjIsActive(vm) && - qemuProcessStartCPUs(driver, vm, conn, - VIR_DOMAIN_RUNNING_UNPAUSED, - QEMU_ASYNC_JOB_NONE) < 0 && - virGetLastError() == NULL) { - qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("resuming after snapshot failed")); + if (resume && virDomainObjIsActive(vm)) { + if (qemuProcessStartCPUs(driver, vm, conn, + VIR_DOMAIN_RUNNING_UNPAUSED, + QEMU_ASYNC_JOB_NONE) < 0 && + virGetLastError() == NULL) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("resuming after snapshot failed")); + goto endjob; + } + + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) && + qemuDomainSnapshotFSThaw(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to thaw domain's file systems")); + } } if (vm) { @@ -9888,7 +9968,15 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA | VIR_DOMAIN_SNAPSHOT_CREATE_HALT | VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY | - VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT, NULL); + VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT | + VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE, NULL); + + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) && + !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("quiesce requires disk-only")); + return NULL; + } if (((flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)) || -- 1.7.3.4
On Tue, Jan 24, 2012 at 18:37:44 +0100, Michal Privoznik wrote:
With this flag, virDomainSnapshotCreate will use fs-freeze and fs-thaw guest agent commands to quiesce guest's disks. --- include/libvirt/libvirt.h.in | 4 ++ src/libvirt.c | 6 ++ src/qemu/qemu_driver.c | 118 ++++++++++++++++++++++++++++++++++++----- 3 files changed, 113 insertions(+), 15 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5e6e488..182065d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3125,6 +3125,10 @@ typedef enum { system checkpoint */ VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT = (1 << 5), /* reuse any existing external files */ + VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE = (1 << 6), /* use guest agent to + quiesce all mounted + file systems within + the domain */
Do we also want to add another flag that would make quiescing optional? That is, use it if it's available but don't fail if it's not.
} virDomainSnapshotCreateFlags;
/* Take a snapshot of the current VM state */ diff --git a/src/libvirt.c b/src/libvirt.c index 96ad3d5..d9b8d88 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16602,6 +16602,12 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot) * inconsistent (as if power had been pulled), and specifying this * with the VIR_DOMAIN_SNAPSHOT_CREATE_HALT flag risks data loss. * + * If @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE, then the + * libvirt will attempt to use guest agent to freeze and thaw all + * file systems in use within domain OS. However, if the guest agent + * is not present, an error is trowed. Moreover, this flags requires
s/trowed/thrown/; s/flags/flag/
+ * VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY to be passed as well. + * * By default, if the snapshot involves external files, and any of the * destination files already exist as a regular file, the snapshot is * rejected to avoid losing contents of those files. However, if diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c81289a..14ad30b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9461,6 +9461,56 @@ static int qemuDomainSnapshotIsAllowed(virDomainObjPtr vm) return 1; }
+static int +qemuDomainSnapshotFSFreeze(struct qemud_driver *driver, + virDomainObjPtr vm) { + qemuDomainObjPrivatePtr priv = vm->privateData; + int freezed; + + if (priv->agentError) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QEMU guest agent is not " + "available due to an error")); + return -1; + } + if (!priv->agent) { + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + return -1; + } + + qemuDomainObjEnterAgent(driver, vm); + freezed = qemuAgentFSFreeze(priv->agent); + qemuDomainObjExitAgent(driver, vm); + + return freezed; +} + +static int +qemuDomainSnapshotFSThaw(struct qemud_driver *driver, + virDomainObjPtr vm) { + qemuDomainObjPrivatePtr priv = vm->privateData; + int thawed; + + if (priv->agentError) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QEMU guest agent is not " + "available due to an error")); + return -1; + } + if (!priv->agent) { + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + return -1; + } + + qemuDomainObjEnterAgent(driver, vm); + thawed = qemuAgentFSThaw(priv->agent); + qemuDomainObjExitAgent(driver, vm); + + return thawed; +} + /* The domain is expected to be locked and inactive. */ static int qemuDomainSnapshotCreateInactive(struct qemud_driver *driver, @@ -9493,6 +9543,13 @@ qemuDomainSnapshotCreateActive(virConnectPtr conn, }
if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) && + (qemuDomainSnapshotFSFreeze(driver, vm) < 0)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to freeze domain's file systems"));
This just masks useful error reported by qemuDomainSnapshotFSFreeze().
+ goto endjob; + } + /* 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. @@ -9532,13 +9589,21 @@ qemuDomainSnapshotCreateActive(virConnectPtr conn, }
cleanup: - if (resume && virDomainObjIsActive(vm) && - qemuProcessStartCPUs(driver, vm, conn, - VIR_DOMAIN_RUNNING_UNPAUSED, - QEMU_ASYNC_JOB_NONE) < 0 && - virGetLastError() == NULL) { - qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("resuming after snapshot failed")); + if (resume && virDomainObjIsActive(vm)) { + if (qemuProcessStartCPUs(driver, vm, conn, + VIR_DOMAIN_RUNNING_UNPAUSED, + QEMU_ASYNC_JOB_NONE) < 0 && + virGetLastError() == NULL) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("resuming after snapshot failed")); + goto endjob; + } + + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) && + qemuDomainSnapshotFSThaw(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to thaw domain's file systems"));
ditto
+ } }
endjob: @@ -9773,6 +9838,13 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) && + (qemuDomainSnapshotFSFreeze(driver, vm) < 0)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to freeze domain's file systems"));
Again.
+ goto endjob; + } + /* In qemu, snapshot_blkdev on a single disk will pause cpus, * but this confuses libvirt since notifications are not given * when qemu resumes. And for multiple disks, libvirt must @@ -9840,13 +9912,21 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn, }
cleanup: - if (resume && virDomainObjIsActive(vm) && - qemuProcessStartCPUs(driver, vm, conn, - VIR_DOMAIN_RUNNING_UNPAUSED, - QEMU_ASYNC_JOB_NONE) < 0 && - virGetLastError() == NULL) { - qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("resuming after snapshot failed")); + if (resume && virDomainObjIsActive(vm)) { + if (qemuProcessStartCPUs(driver, vm, conn, + VIR_DOMAIN_RUNNING_UNPAUSED, + QEMU_ASYNC_JOB_NONE) < 0 && + virGetLastError() == NULL) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("resuming after snapshot failed")); + goto endjob; + } + + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) && + qemuDomainSnapshotFSThaw(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to thaw domain's file systems"));
And once more.
+ } }
if (vm) { @@ -9888,7 +9968,15 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA | VIR_DOMAIN_SNAPSHOT_CREATE_HALT | VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY | - VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT, NULL); + VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT | + VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE, NULL); + + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) && + !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("quiesce requires disk-only")); + return NULL; + }
if (((flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)) ||
Jirka
On 24.01.2012 19:49, Jiri Denemark wrote:
On Tue, Jan 24, 2012 at 18:37:44 +0100, Michal Privoznik wrote:
With this flag, virDomainSnapshotCreate will use fs-freeze and fs-thaw guest agent commands to quiesce guest's disks. --- include/libvirt/libvirt.h.in | 4 ++ src/libvirt.c | 6 ++ src/qemu/qemu_driver.c | 118 ++++++++++++++++++++++++++++++++++++----- 3 files changed, 113 insertions(+), 15 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5e6e488..182065d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3125,6 +3125,10 @@ typedef enum { system checkpoint */ VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT = (1 << 5), /* reuse any existing external files */ + VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE = (1 << 6), /* use guest agent to + quiesce all mounted + file systems within + the domain */
Do we also want to add another flag that would make quiescing optional? That is, use it if it's available but don't fail if it's not.
If this is ever needed we can add it then.
@@ -9493,6 +9543,13 @@ qemuDomainSnapshotCreateActive(virConnectPtr conn, }
if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) && + (qemuDomainSnapshotFSFreeze(driver, vm) < 0)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to freeze domain's file systems"));
This just masks useful error reported by qemuDomainSnapshotFSFreeze().
In fact, I just realized, this is dead code. Since we are checking for DISK_ONLY flag which implies calling qemuDomainSnapshotCreateDiskActive() we will never get this condition to evaluate as true. I'll drop it in v2.
to cmdSnapshotCreate and cmdSnapshotCreateAs. --- tools/virsh.c | 6 ++++++ tools/virsh.pod | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 5560988..999941c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -14662,6 +14662,7 @@ static const vshCmdOptDef opts_snapshot_create[] = { {"halt", VSH_OT_BOOL, 0, N_("halt domain after snapshot is created")}, {"disk-only", VSH_OT_BOOL, 0, N_("capture disk state but not vm state")}, {"reuse-external", VSH_OT_BOOL, 0, N_("reuse any existing external files")}, + {"quiesce", VSH_OT_BOOL, 0, N_("quiesce guest's file systems")}, {NULL, 0, 0, NULL} }; @@ -14686,6 +14687,8 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY; if (vshCommandOptBool(cmd, "reuse-external")) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT; + if (vshCommandOptBool(cmd, "quiesce")) + flags |= VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE; if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; @@ -14795,6 +14798,7 @@ static const vshCmdOptDef opts_snapshot_create_as[] = { {"halt", VSH_OT_BOOL, 0, N_("halt domain after snapshot is created")}, {"disk-only", VSH_OT_BOOL, 0, N_("capture disk state but not vm state")}, {"reuse-external", VSH_OT_BOOL, 0, N_("reuse any existing external files")}, + {"quiesce", VSH_OT_BOOL, 0, N_("quiesce guest's file systems")}, {"diskspec", VSH_OT_ARGV, 0, N_("disk attributes: disk[,snapshot=type][,driver=type][,file=name]")}, {NULL, 0, 0, NULL} @@ -14820,6 +14824,8 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY; if (vshCommandOptBool(cmd, "reuse-external")) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT; + if (vshCommandOptBool(cmd, "quiesce")) + flags |= VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE; if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; diff --git a/tools/virsh.pod b/tools/virsh.pod index 72c6d8f..b1f75de 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2046,7 +2046,8 @@ used to represent properties of snapshots. =over 4 =item B<snapshot-create> I<domain> [I<xmlfile>] {[I<--redefine> [I<--current>]] -| [I<--no-metadata>] [I<--halt>] [I<--disk-only>] [I<--reuse-external]} +| [I<--no-metadata>] [I<--halt>] [I<--disk-only>] [I<--reuse-external] +[I<--quiesce>]} Create a snapshot for domain I<domain> with the properties specified in I<xmlfile>. Normally, the only properties settable for a domain snapshot @@ -2088,6 +2089,10 @@ external snapshot with a destination of an existing file, then the existing file is truncated and reused; otherwise, a snapshot is refused to avoid losing contents of the existing files. +If I<--quiesce> is specified, libvirt will try to use guest agent +to freeze and unfreeze domain's mounted file systems. However, +currently this requires I<--disk-only> to be passed as well. + Existence of snapshot metadata will prevent attempts to B<undefine> a persistent domain. However, for transient domains, snapshot metadata is silently lost when the domain quits running (whether @@ -2095,7 +2100,8 @@ by command such as B<destroy> or by internal guest action). =item B<snapshot-create-as> I<domain> {[I<--print-xml>] | [I<--no-metadata>] [I<--halt>] [I<--reuse-existing>]} [I<name>] -[I<description>] [I<--disk-only> [[I<--diskspec>] B<diskspec>]...] +[I<description>] [I<--disk-only> [I<--quiesce>] +[[I<--diskspec>] B<diskspec>]...] Create a snapshot for domain I<domain> with the given <name> and <description>; if either value is omitted, libvirt will choose a @@ -2123,6 +2129,10 @@ option requests an external snapshot with a destination of an existing file, then the existing file is truncated and reused; otherwise, a snapshot is refused to avoid losing contents of the existing files. +If I<--quiesce> is specified, libvirt will try to use guest agent +to freeze and unfreeze domain's mounted file systems. However, +currently this requires I<--disk-only> to be passed as well. + If I<--no-metadata> is specified, then the snapshot data is created, but any metadata is immediately discarded (that is, libvirt does not treat the snapshot as current, and cannot revert to the snapshot -- 1.7.3.4
On Tue, Jan 24, 2012 at 18:37:45 +0100, Michal Privoznik wrote:
to cmdSnapshotCreate and cmdSnapshotCreateAs. --- tools/virsh.c | 6 ++++++ tools/virsh.pod | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 5560988..999941c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -14662,6 +14662,7 @@ static const vshCmdOptDef opts_snapshot_create[] = { {"halt", VSH_OT_BOOL, 0, N_("halt domain after snapshot is created")}, {"disk-only", VSH_OT_BOOL, 0, N_("capture disk state but not vm state")}, {"reuse-external", VSH_OT_BOOL, 0, N_("reuse any existing external files")}, + {"quiesce", VSH_OT_BOOL, 0, N_("quiesce guest's file systems")}, {NULL, 0, 0, NULL} };
@@ -14686,6 +14687,8 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY; if (vshCommandOptBool(cmd, "reuse-external")) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT; + if (vshCommandOptBool(cmd, "quiesce")) + flags |= VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE;
if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; @@ -14795,6 +14798,7 @@ static const vshCmdOptDef opts_snapshot_create_as[] = { {"halt", VSH_OT_BOOL, 0, N_("halt domain after snapshot is created")}, {"disk-only", VSH_OT_BOOL, 0, N_("capture disk state but not vm state")}, {"reuse-external", VSH_OT_BOOL, 0, N_("reuse any existing external files")}, + {"quiesce", VSH_OT_BOOL, 0, N_("quiesce guest's file systems")}, {"diskspec", VSH_OT_ARGV, 0, N_("disk attributes: disk[,snapshot=type][,driver=type][,file=name]")}, {NULL, 0, 0, NULL} @@ -14820,6 +14824,8 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY; if (vshCommandOptBool(cmd, "reuse-external")) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT; + if (vshCommandOptBool(cmd, "quiesce")) + flags |= VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE;
if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; diff --git a/tools/virsh.pod b/tools/virsh.pod index 72c6d8f..b1f75de 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2046,7 +2046,8 @@ used to represent properties of snapshots. =over 4
=item B<snapshot-create> I<domain> [I<xmlfile>] {[I<--redefine> [I<--current>]] -| [I<--no-metadata>] [I<--halt>] [I<--disk-only>] [I<--reuse-external]} +| [I<--no-metadata>] [I<--halt>] [I<--disk-only>] [I<--reuse-external] +[I<--quiesce>]}
Create a snapshot for domain I<domain> with the properties specified in I<xmlfile>. Normally, the only properties settable for a domain snapshot @@ -2088,6 +2089,10 @@ external snapshot with a destination of an existing file, then the existing file is truncated and reused; otherwise, a snapshot is refused to avoid losing contents of the existing files.
+If I<--quiesce> is specified, libvirt will try to use guest agent +to freeze and unfreeze domain's mounted file systems. However, +currently this requires I<--disk-only> to be passed as well. +
I'm not a native speaker but I feel like "libvirt will try to use guest agent" suggests that the snapshot will still be created even though guest agent couldn't be used for freezing guest's file systems. I'd rather be explicit about the fact that no guest agent implies no snapshot.
Existence of snapshot metadata will prevent attempts to B<undefine> a persistent domain. However, for transient domains, snapshot metadata is silently lost when the domain quits running (whether @@ -2095,7 +2100,8 @@ by command such as B<destroy> or by internal guest action).
=item B<snapshot-create-as> I<domain> {[I<--print-xml>] | [I<--no-metadata>] [I<--halt>] [I<--reuse-existing>]} [I<name>] -[I<description>] [I<--disk-only> [[I<--diskspec>] B<diskspec>]...] +[I<description>] [I<--disk-only> [I<--quiesce>] +[[I<--diskspec>] B<diskspec>]...]
Create a snapshot for domain I<domain> with the given <name> and <description>; if either value is omitted, libvirt will choose a @@ -2123,6 +2129,10 @@ option requests an external snapshot with a destination of an existing file, then the existing file is truncated and reused; otherwise, a snapshot is refused to avoid losing contents of the existing files.
+If I<--quiesce> is specified, libvirt will try to use guest agent +to freeze and unfreeze domain's mounted file systems. However, +currently this requires I<--disk-only> to be passed as well. +
And here as well.
If I<--no-metadata> is specified, then the snapshot data is created, but any metadata is immediately discarded (that is, libvirt does not treat the snapshot as current, and cannot revert to the snapshot
Jirka
On 24.01.2012 19:57, Jiri Denemark wrote:
On Tue, Jan 24, 2012 at 18:37:45 +0100, Michal Privoznik wrote:
to cmdSnapshotCreate and cmdSnapshotCreateAs. --- tools/virsh.c | 6 ++++++ tools/virsh.pod | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod index 72c6d8f..b1f75de 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2046,7 +2046,8 @@ used to represent properties of snapshots. =over 4
=item B<snapshot-create> I<domain> [I<xmlfile>] {[I<--redefine> [I<--current>]] -| [I<--no-metadata>] [I<--halt>] [I<--disk-only>] [I<--reuse-external]} +| [I<--no-metadata>] [I<--halt>] [I<--disk-only>] [I<--reuse-external] +[I<--quiesce>]}
Create a snapshot for domain I<domain> with the properties specified in I<xmlfile>. Normally, the only properties settable for a domain snapshot @@ -2088,6 +2089,10 @@ external snapshot with a destination of an existing file, then the existing file is truncated and reused; otherwise, a snapshot is refused to avoid losing contents of the existing files.
+If I<--quiesce> is specified, libvirt will try to use guest agent +to freeze and unfreeze domain's mounted file systems. However, +currently this requires I<--disk-only> to be passed as well. +
I'm not a native speaker but I feel like "libvirt will try to use guest agent" suggests that the snapshot will still be created even though guest agent couldn't be used for freezing guest's file systems. I'd rather be explicit about the fact that no guest agent implies no snapshot.
Okay, reworded in v2.
participants (2)
-
Jiri Denemark -
Michal Privoznik