[libvirt] [PATCH v6 0/6] Expose FSFreeze/FSThaw within the guest as API
Hello, This is patchset v6 to add FSFreeze/FSThaw API for custom disk snapshotting. Changes since v5: * replace disks and ndisks parameter with mountpoints and nmountpoints to specify filesystem to be frozen/thawed, so that we can specify non-disk-backed filesystems * add PATCH 6 to support 'mountpoints' argument for 'guest-fsfreeze-freeze' command of qemu-guest-agent. (This requires a patch for qemu-guest-agent, which isn't merged yet: https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg04434.html ) * drop nested fsfreeze request feature * change API version to 1.2.5 (v5: https://www.redhat.com/archives/libvir-list/2014-April/msg00140.html ) === Description === Currently FSFreeze and FSThaw are supported by qemu guest agent and they are used internally in snapshot-create command with --quiesce option. However, when users want to utilize the native snapshot feature of storage devices (such as LVM over iSCSI, enterprise storage appliances, etc.), they need to issue fsfreeze command separately from libvirt-driven snapshots. (OpenStack cinder provides these storages' snapshot feature, but it cannot quiesce the guest filesystems automatically for now.) Although virDomainQemuGuestAgent() API could be used for this purpose, it is only for debugging and is not supported officially. This patchset adds virDomainFSFreeze()/virDomainFSThaw() APIs and virsh domfsfreeze/domfsthaw commands to enable the users to freeze and thaw domain's filesystems cleanly. <updated> The APIs take disks and ndisks parameters, which is a list of mountpoints of filesystems to be frozen/thawed. If the option is not provided, or guest agent doesn't support the 'mountpoints' argument, every mounted filesystem is frozen/thawed. </updated> The APIs have flags option currently unsupported for future extension. --- Tomoki Sekiyama (6): Introduce virDomainFSFreeze() and virDomainFSThaw() public API remote: Implement virDomainFSFreeze and virDomainFSThaw qemu: track quiesced status in qemuDomainSnapshotFSFreeze qemu: Implement virDomainFSFreeze and virDomainFSThaw virsh: Expose new virDomainFSFreeze and virDomainFSThaw API qemu: Support mountpoints option of guest-fsfreeze-freeze include/libvirt/libvirt.h.in | 10 +++ src/access/viraccessperm.c | 2 - src/access/viraccessperm.h | 6 ++ src/driver.h | 14 ++++ src/libvirt.c | 93 ++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ src/qemu/qemu_agent.c | 47 +++++++++++- src/qemu/qemu_agent.h | 3 + src/qemu/qemu_domain.c | 5 + src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 160 ++++++++++++++++++++++++++++++++++++++---- src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 38 +++++++++- src/remote_protocol-structs | 18 +++++ src/rpc/gendispatch.pl | 2 + tests/qemuagenttest.c | 8 +- tools/virsh-domain.c | 130 ++++++++++++++++++++++++++++++++++ tools/virsh.pod | 23 ++++++ 18 files changed, 544 insertions(+), 25 deletions(-)
These will freeze and thaw filesystems within guest specified by @mountpoints parameters. The parameters can be NULL and 0, then the all mounted filesystes are frozen or thawed. @flags parameter, which are currently not used, is for future extensions. Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> --- include/libvirt/libvirt.h.in | 10 +++++ src/driver.h | 14 ++++++ src/libvirt.c | 93 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++ 4 files changed, 123 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 930b7e8..878ddc5 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -5277,6 +5277,16 @@ int virDomainFSTrim(virDomainPtr dom, unsigned long long minimum, unsigned int flags); +int virDomainFSFreeze(virDomainPtr dom, + const char **mountpoints, + unsigned int nmountpoints, + unsigned int flags); + +int virDomainFSThaw(virDomainPtr dom, + const char **mountpoints, + unsigned int nmountpoints, + unsigned int flags); + /** * virSchedParameterType: * diff --git a/src/driver.h b/src/driver.h index 729e743..502f30e 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1149,6 +1149,18 @@ typedef int unsigned int flags, int cancelled); +typedef int +(*virDrvDomainFSFreeze)(virDomainPtr dom, + const char **mountpoints, + unsigned int nmountpoints, + unsigned int flags); + +typedef int +(*virDrvDomainFSThaw)(virDomainPtr dom, + const char **mountpoints, + unsigned int nmountpoints, + unsigned int flags); + typedef struct _virDriver virDriver; typedef virDriver *virDriverPtr; @@ -1363,6 +1375,8 @@ struct _virDriver { virDrvDomainMigrateFinish3Params domainMigrateFinish3Params; virDrvDomainMigrateConfirm3Params domainMigrateConfirm3Params; virDrvConnectGetCPUModelNames connectGetCPUModelNames; + virDrvDomainFSFreeze domainFSFreeze; + virDrvDomainFSThaw domainFSThaw; }; diff --git a/src/libvirt.c b/src/libvirt.c index b6c99c5..4dfacf6 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20682,3 +20682,96 @@ virDomainFSTrim(virDomainPtr dom, virDispatchError(dom->conn); return -1; } + +/** + * virDomainFSFreeze: + * @dom: a domain object + * @mountpoints: list of mount points to be frozen + * @nmountpoints: the number of mount points specified in @mountpoints + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * Freeze specified filesystems within the guest (hence guest agent + * may be required depending on hypervisor used). If @mountpoints is NULL or + * @nmountpoints is 0, every mounted filesystem on the guest is frozen. + * In some environments (e.g. QEMU guest with guest agent which doesn't + * support mountpoints argument), @mountpoints may be ignored. + * + * Returns the number of frozen filesystems on success, -1 otherwise. + */ +int +virDomainFSFreeze(virDomainPtr dom, + const char **mountpoints, + unsigned int nmountpoints, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(dom, "mountpoints=%p, nmountpoints=%d, flags=%x", + mountpoints, nmountpoints, flags); + + virResetLastError(); + + virCheckDomainReturn(dom, -1); + virCheckReadOnlyGoto(dom->conn->flags, error); + if (nmountpoints) + virCheckNonNullArgGoto(mountpoints, error); + else + virCheckNullArgGoto(mountpoints, error); + + if (dom->conn->driver->domainFSFreeze) { + int ret = dom->conn->driver->domainFSFreeze( + dom, mountpoints, nmountpoints, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(dom->conn); + return -1; +} + +/** + * virDomainFSThaw: + * @dom: a domain object + * @mountpoints: list of mount points to be thawed + * @nmountpoints: the number of mount points specified in @mountpoints + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * Thaw specified filesystems within the guest. If @mountpoints is NULL or + * @nmountpoints is 0, every mounted filesystem on the guest is thawed. + * In some drivers (e.g. QEMU driver), @mountpoints may always be ignored. + * + * Returns the number of thawed filesystems on success, -1 otherwise. + */ +int +virDomainFSThaw(virDomainPtr dom, + const char **mountpoints, + unsigned int nmountpoints, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(dom, "flags=%x", flags); + + virResetLastError(); + + virCheckDomainReturn(dom, -1); + virCheckReadOnlyGoto(dom->conn->flags, error); + if (nmountpoints) + virCheckNonNullArgGoto(mountpoints, error); + else + virCheckNullArgGoto(mountpoints, error); + + if (dom->conn->driver->domainFSThaw) { + int ret = dom->conn->driver->domainFSThaw( + dom, mountpoints, nmountpoints, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(dom->conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 9ab0c92..f8113f4 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -650,5 +650,11 @@ LIBVIRT_1.2.3 { virDomainCoreDumpWithFormat; } LIBVIRT_1.2.1; +LIBVIRT_1.2.5 { + global: + virDomainFSFreeze; + virDomainFSThaw; +} LIBVIRT_1.2.3; + # .... define new API here using predicted next version number ....
On Tue, Apr 29, 2014 at 08:03:57PM -0400, Tomoki Sekiyama wrote:
These will freeze and thaw filesystems within guest specified by @mountpoints parameters. The parameters can be NULL and 0, then the all mounted filesystes are frozen or thawed. @flags parameter, which are currently not used, is for future extensions.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
diff --git a/src/libvirt.c b/src/libvirt.c index b6c99c5..4dfacf6 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20682,3 +20682,96 @@ virDomainFSTrim(virDomainPtr dom, virDispatchError(dom->conn); return -1; } + +/** + * virDomainFSFreeze: + * @dom: a domain object + * @mountpoints: list of mount points to be frozen + * @nmountpoints: the number of mount points specified in @mountpoints + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * Freeze specified filesystems within the guest (hence guest agent + * may be required depending on hypervisor used). If @mountpoints is NULL or + * @nmountpoints is 0, every mounted filesystem on the guest is frozen. + * In some environments (e.g. QEMU guest with guest agent which doesn't + * support mountpoints argument), @mountpoints may be ignored.
We shouldn't be ignoring the mountpoints argument. I'd reword this as "Not all drivers will support freezing individual mount points, and may return VIR_ERR_ARGUMENT_UNSUPPORTED if @mountpoints is non-NULL"
+/** + * virDomainFSThaw: + * @dom: a domain object + * @mountpoints: list of mount points to be thawed + * @nmountpoints: the number of mount points specified in @mountpoints + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * Thaw specified filesystems within the guest. If @mountpoints is NULL or + * @nmountpoints is 0, every mounted filesystem on the guest is thawed. + * In some drivers (e.g. QEMU driver), @mountpoints may always be ignored.
Again, clarify the text in the same way.
+ * + * Returns the number of thawed filesystems on success, -1 otherwise. + */ +int +virDomainFSThaw(virDomainPtr dom, + const char **mountpoints, + unsigned int nmountpoints, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(dom, "flags=%x", flags);
Should include mountpoints/nmountpoints in debug line. Those nit-picks aside, this API looks good. Regards, 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 :|
Hi, thank you for the review. On 4/30/14 11:26 , "Daniel P. Berrange" <berrange@redhat.com> wrote:
On Tue, Apr 29, 2014 at 08:03:57PM -0400, Tomoki Sekiyama wrote:
These will freeze and thaw filesystems within guest specified by @mountpoints parameters. The parameters can be NULL and 0, then the all mounted filesystes are frozen or thawed. @flags parameter, which are currently not used, is for future extensions.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
diff --git a/src/libvirt.c b/src/libvirt.c index b6c99c5..4dfacf6 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20682,3 +20682,96 @@ virDomainFSTrim(virDomainPtr dom, virDispatchError(dom->conn); return -1; } + +/** + * virDomainFSFreeze: + * @dom: a domain object + * @mountpoints: list of mount points to be frozen + * @nmountpoints: the number of mount points specified in @mountpoints + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * Freeze specified filesystems within the guest (hence guest agent + * may be required depending on hypervisor used). If @mountpoints is NULL or + * @nmountpoints is 0, every mounted filesystem on the guest is frozen. + * In some environments (e.g. QEMU guest with guest agent which doesn't + * support mountpoints argument), @mountpoints may be ignored.
We shouldn't be ignoring the mountpoints argument. I'd reword this as
"Not all drivers will support freezing individual mount points, and may return VIR_ERR_ARGUMENT_UNSUPPORTED if @mountpoints is non-NULL"
I will update to this, and others too. Thanks.
+/** + * virDomainFSThaw: + * @dom: a domain object + * @mountpoints: list of mount points to be thawed + * @nmountpoints: the number of mount points specified in @mountpoints + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * Thaw specified filesystems within the guest. If @mountpoints is NULL or + * @nmountpoints is 0, every mounted filesystem on the guest is thawed. + * In some drivers (e.g. QEMU driver), @mountpoints may always be ignored.
Again, clarify the text in the same way.
+ * + * Returns the number of thawed filesystems on success, -1 otherwise. + */ +int +virDomainFSThaw(virDomainPtr dom, + const char **mountpoints, + unsigned int nmountpoints, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(dom, "flags=%x", flags);
Should include mountpoints/nmountpoints in debug line.
Those nit-picks aside, this API looks good.
Regards, Daniel
Regards, Tomoki Sekiyama
New rules are added in fixup_name in gendispatch.pl to keep the name FSFreeze and FSThaw. This adds a new ACL permission 'fs_freeze', which is also applied to VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag. Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> --- src/access/viraccessperm.c | 2 +- src/access/viraccessperm.h | 6 ++++++ src/remote/remote_driver.c | 2 ++ src/remote/remote_protocol.x | 38 ++++++++++++++++++++++++++++++++++++-- src/remote_protocol-structs | 18 ++++++++++++++++++ src/rpc/gendispatch.pl | 2 ++ 6 files changed, 65 insertions(+), 3 deletions(-) diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c index d517c66..462f46c 100644 --- a/src/access/viraccessperm.c +++ b/src/access/viraccessperm.c @@ -39,7 +39,7 @@ VIR_ENUM_IMPL(virAccessPermDomain, "start", "stop", "reset", "save", "delete", "migrate", "snapshot", "suspend", "hibernate", "core_dump", "pm_control", - "init_control", "inject_nmi", "send_input", "send_signal", "fs_trim", + "init_control", "inject_nmi", "send_input", "send_signal", "fs_trim", "fs_freeze", "block_read", "block_write", "mem_read", "open_graphics", "open_device", "screenshot", "open_namespace"); diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h index 6d14f05..ac48d70 100644 --- a/src/access/viraccessperm.h +++ b/src/access/viraccessperm.h @@ -242,6 +242,12 @@ typedef enum { */ VIR_ACCESS_PERM_DOMAIN_FS_TRIM, /* Issue TRIM to guest filesystems */ + /** + * @desc: Freeze and thaw domain filesystems + * @message: Freezing and thawing domain filesystems require authorization + */ + VIR_ACCESS_PERM_DOMAIN_FS_FREEZE, /* Freeze and thaw guest filesystems */ + /* Peeking at guest */ /** diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index d9be756..4c3b38a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7768,6 +7768,8 @@ static virDriver remote_driver = { .domainMigrateFinish3Params = remoteDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = remoteDomainMigrateConfirm3Params, /* 1.1.0 */ .connectGetCPUModelNames = remoteConnectGetCPUModelNames, /* 1.1.3 */ + .domainFSFreeze = remoteDomainFSFreeze, /* 1.2.5 */ + .domainFSThaw = remoteDomainFSThaw, /* 1.2.5 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 6c445cc..608490c 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -235,6 +235,9 @@ const REMOTE_DOMAIN_JOB_STATS_MAX = 64; /* Upper limit on number of CPU models */ const REMOTE_CONNECT_CPU_MODELS_MAX = 8192; +/* Upper limit on number of mountpoints to frozen */ +const REMOTE_DOMAIN_FSFREEZE_MOUNTPOINTS_MAX = 256; + /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN]; @@ -2959,6 +2962,25 @@ struct remote_network_event_lifecycle_msg { int detail; }; +struct remote_domain_fsfreeze_args { + remote_nonnull_domain dom; + remote_nonnull_string mountpoints<REMOTE_DOMAIN_FSFREEZE_MOUNTPOINTS_MAX>; /* (const char **) */ + unsigned int flags; +}; + +struct remote_domain_fsfreeze_ret { + int filesystems; +}; + +struct remote_domain_fsthaw_args { + remote_nonnull_domain dom; + remote_nonnull_string mountpoints<REMOTE_DOMAIN_FSFREEZE_MOUNTPOINTS_MAX>; /* (const char **) */ + unsigned int flags; +}; + +struct remote_domain_fsthaw_ret { + int filesystems; +}; /*----- Protocol. -----*/ @@ -4289,7 +4311,7 @@ enum remote_procedure { /** * @generate: both * @acl: domain:snapshot - * @acl: domain:write:VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE + * @acl: domain:fs_freeze:VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE */ REMOTE_PROC_DOMAIN_SNAPSHOT_CREATE_XML = 185, @@ -5275,5 +5297,17 @@ enum remote_procedure { * @generate: both * @acl: domain:core_dump */ - REMOTE_PROC_DOMAIN_CORE_DUMP_WITH_FORMAT = 334 + REMOTE_PROC_DOMAIN_CORE_DUMP_WITH_FORMAT = 334, + + /** + * @generate: both + * @acl: domain:fs_freeze + */ + REMOTE_PROC_DOMAIN_FSFREEZE = 335, + + /** + * @generate: both + * @acl: domain:fs_freeze + */ + REMOTE_PROC_DOMAIN_FSTHAW = 336 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 456d0da..15c95ca 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2427,6 +2427,22 @@ struct remote_network_event_lifecycle_msg { int event; int detail; }; +struct remote_domain_fsfreeze_args { + remote_nonnull_domain dom; + struct { + u_int mountpoints_len; + remote_nonnull_string * mountpoints_val; + } mountpoints; + u_int flags; +}; +struct remote_domain_fsthaw_args { + remote_nonnull_domain dom; + struct { + u_int mountpoints_len; + remote_nonnull_string * mountpoints_val; + } mountpoints; + u_int flags; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -2762,4 +2778,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_CALLBACK_PMSUSPEND_DISK = 332, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVED = 333, REMOTE_PROC_DOMAIN_CORE_DUMP_WITH_FORMAT = 334, + REMOTE_PROC_DOMAIN_FSFREEZE = 335, + REMOTE_PROC_DOMAIN_FSTHAW = 336, }; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index b76bbac..9cd620e 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -64,6 +64,8 @@ sub fixup_name { $name =~ s/Nmi$/NMI/; $name =~ s/Pm/PM/; $name =~ s/Fstrim$/FSTrim/; + $name =~ s/Fsfreeze$/FSFreeze/; + $name =~ s/Fsthaw$/FSThaw/; $name =~ s/Scsi/SCSI/; $name =~ s/Wwn$/WWN/;
On Tue, Apr 29, 2014 at 08:04:04PM -0400, Tomoki Sekiyama wrote:
New rules are added in fixup_name in gendispatch.pl to keep the name FSFreeze and FSThaw. This adds a new ACL permission 'fs_freeze', which is also applied to VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> --- src/access/viraccessperm.c | 2 +- src/access/viraccessperm.h | 6 ++++++ src/remote/remote_driver.c | 2 ++ src/remote/remote_protocol.x | 38 ++++++++++++++++++++++++++++++++++++-- src/remote_protocol-structs | 18 ++++++++++++++++++ src/rpc/gendispatch.pl | 2 ++ 6 files changed, 65 insertions(+), 3 deletions(-)
ACK Regards, 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 :|
Adds 'quiesced' status into qemuDomainObjPrivate that tracks whether FSFreeze is requested in the domain. It modifies error code from qemuDomainSnapshotFSFreeze and qemuDomainSnapshotFSThaw, so that a caller can know whether the command is actually sent to the guest agent. If the error is caused before sending a freeze command, a counterpart thaw command shouldn't be sent either, not to confuse fsfreeze status tracking. Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> --- src/qemu/qemu_domain.c | 5 +++ src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 70 +++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 64 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8fa58f3..1dd507f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -357,6 +357,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) virBufferAddLit(buf, "</devices>\n"); } + if (priv->quiesced) + virBufferAddLit(buf, "<quiesced/>\n"); + return 0; } @@ -518,6 +521,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data) } VIR_FREE(nodes); + priv->quiesced = virXPathBoolean("boolean(./quiesced)", ctxt) == 1; + return 0; error: diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 31611b5..ab27f15 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -176,6 +176,8 @@ struct _qemuDomainObjPrivate { char **qemuDevices; /* NULL-terminated list of devices aliases known to QEMU */ bool hookRun; /* true if there was a hook run over this domain */ + + bool quiesced; /* true if filesystems are quiesced */ }; typedef enum { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 75cf8cb..c3e0682 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12060,32 +12060,62 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, } +/* Return -1 if request is not sent to agent due to misconfig, -2 if request + * is sent but failed, and number of frozen filesystems on success. If -2 is + * returned, FSThaw should be called revert the quiesced status. */ static int -qemuDomainSnapshotFSFreeze(virDomainObjPtr vm) +qemuDomainSnapshotFSFreeze(virQEMUDriverPtr driver, + virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverConfigPtr cfg; int freezed; + if (priv->quiesced) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is already quiesced")); + return -1; + } + if (!qemuDomainAgentAvailable(priv, true)) return -1; + priv->quiesced = true; + + cfg = virQEMUDriverGetConfig(driver); + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) { + priv->quiesced = false; + virObjectUnref(cfg); + return -1; + } + virObjectUnref(cfg); + qemuDomainObjEnterAgent(vm); freezed = qemuAgentFSFreeze(priv->agent); qemuDomainObjExitAgent(vm); - - return freezed; + return freezed < 0 ? -2 : freezed; } +/* Return -1 on error, otherwise number of thawed filesystems. */ static int -qemuDomainSnapshotFSThaw(virDomainObjPtr vm, bool report) +qemuDomainSnapshotFSThaw(virQEMUDriverPtr driver, + virDomainObjPtr vm, + bool report) { qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverConfigPtr cfg; int thawed; virErrorPtr err = NULL; if (!qemuDomainAgentAvailable(priv, report)) return -1; + if (!priv->quiesced && report) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not quiesced")); + return -1; + } + qemuDomainObjEnterAgent(vm); if (!report) err = virSaveLastError(); @@ -12095,6 +12125,19 @@ qemuDomainSnapshotFSThaw(virDomainObjPtr vm, bool report) qemuDomainObjExitAgent(vm); virFreeError(err); + + if (!report || thawed >= 0) { + priv->quiesced = false; + + cfg = virQEMUDriverGetConfig(driver); + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) { + /* Revert the statuses when we failed to save them. */ + priv->quiesced = true; + thawed = -1; + } + virObjectUnref(cfg); + } + return thawed; } @@ -13092,17 +13135,18 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, goto cleanup; /* If quiesce was requested, then issue a freeze command, and a - * counterpart thaw command, no matter what. The command will - * fail if the guest is paused or the guest agent is not - * running. */ + * counterpart thaw command when it is actually sent to agent. + * The command will fail if the guest is paused or the guest agent + * is not running, or is already quiesced. */ if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) { - if (qemuDomainSnapshotFSFreeze(vm) < 0) { - /* helper reported the error */ - thaw = -1; + int freeze = qemuDomainSnapshotFSFreeze(driver, vm); + if (freeze < 0) { + /* the helper reported the error */ + if (freeze == -2) + thaw = -1; /* the command is sent but agent failed */ goto endjob; - } else { - thaw = 1; } + thaw = 1; } /* We need to track what state the guest is in, since taking the @@ -13243,7 +13287,7 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, goto cleanup; } if (vm && thaw != 0 && - qemuDomainSnapshotFSThaw(vm, thaw > 0) < 0) { + qemuDomainSnapshotFSThaw(driver, vm, thaw > 0) < 0) { /* helper reported the error, if it was needed */ if (thaw > 0) ret = -1;
Use qemuDomainSnapshotFSFreeze() and qemuDomainSnapshotFSFThaw() which are already implemented for snapshot quiescing. Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> --- src/qemu/qemu_driver.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c3e0682..6bfd508 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16528,6 +16528,93 @@ qemuConnectGetCPUModelNames(virConnectPtr conn, } +static int +qemuDomainFSFreeze(virDomainPtr dom, + const char **mountpoints, + unsigned int nmountpoints, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + + virCheckFlags(0, -1); + + if (mountpoints || nmountpoints) + VIR_INFO("mountpoints option is not supported and ignored for now"); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainFSFreezeEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + + ret = qemuDomainSnapshotFSFreeze(driver, vm); + if (ret == -2) { + qemuDomainSnapshotFSThaw(driver, vm, false); + ret = -1; + } + + endjob: + if (!qemuDomainObjEndJob(driver, vm)) + vm = NULL; + + cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + + +static int +qemuDomainFSThaw(virDomainPtr dom, + const char **mountpoints ATTRIBUTE_UNUSED, + unsigned int nmountpoints ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + + virCheckFlags(0, -1); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainFSThawEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + + ret = qemuDomainSnapshotFSThaw(driver, vm, true); + + endjob: + if (!qemuDomainObjEndJob(driver, vm)) + vm = NULL; + + cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = QEMU_DRIVER_NAME, @@ -16718,6 +16805,8 @@ static virDriver qemuDriver = { .domainMigrateFinish3Params = qemuDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = qemuDomainMigrateConfirm3Params, /* 1.1.0 */ .connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.3 */ + .domainFSFreeze = qemuDomainFSFreeze, /* 1.2.5 */ + .domainFSThaw = qemuDomainFSThaw, /* 1.2.5 */ };
On Tue, Apr 29, 2014 at 08:04:18PM -0400, Tomoki Sekiyama wrote:
Use qemuDomainSnapshotFSFreeze() and qemuDomainSnapshotFSFThaw() which are already implemented for snapshot quiescing.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> --- src/qemu/qemu_driver.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c3e0682..6bfd508 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16528,6 +16528,93 @@ qemuConnectGetCPUModelNames(virConnectPtr conn, }
+static int +qemuDomainFSFreeze(virDomainPtr dom, + const char **mountpoints, + unsigned int nmountpoints, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + + virCheckFlags(0, -1); + + if (mountpoints || nmountpoints) + VIR_INFO("mountpoints option is not supported and ignored for now");
We should report VIR_ERR_ARGUMENT_UNSUPPORTED and return -1 to the caller
+static int +qemuDomainFSThaw(virDomainPtr dom, + const char **mountpoints ATTRIBUTE_UNUSED, + unsigned int nmountpoints ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + + virCheckFlags(0, -1);
Same comment about VIR_ERR_ARGUMENT_UNSUPPORTED for nmountpoints being non-NULL. Regards, 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 :|
These are exposed under domfsfreeze command and domfsthaw command. Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> --- tools/virsh-domain.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 23 +++++++++ 2 files changed, 153 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 73414f8..32d52ac 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11391,6 +11391,124 @@ cmdDomFSTrim(vshControl *ctl, const vshCmd *cmd) return ret; } +static const vshCmdInfo info_domfsfreeze[] = { + {.name = "help", + .data = N_("Freeze domain's mounted filesystems.") + }, + {.name = "desc", + .data = N_("Freeze domain's mounted filesystems.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_domfsfreeze[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "mountpoints", + .type = VSH_OT_DATA, + .help = N_("comma separated list of mountpoints to be frozen") + }, + {.name = NULL} +}; +static bool +cmdDomFSFreeze(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + int ret = -1; + unsigned int flags = 0; + const char *mountpoints_string = NULL; + char **list = NULL; /* tokenized mountpoints_string */ + int nmountpoints = 0; + size_t i; + + ignore_value(vshCommandOptString(cmd, "mountpoints", &mountpoints_string)); + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (mountpoints_string) { + if ((nmountpoints = vshStringToArray(mountpoints_string, &list)) < 0) + goto cleanup; + } + + ret = virDomainFSFreeze(dom, (const char **)list, nmountpoints, flags); + if (ret < 0) { + vshError(ctl, _("Unable to freeze filesystems")); + goto cleanup; + } + + vshPrint(ctl, _("Froze %d filesystem(s)\n"), ret); + + cleanup: + for (i = 0; i < nmountpoints; i++) + VIR_FREE(list[i]); + VIR_FREE(list); + virDomainFree(dom); + return ret >= 0; +} + +static const vshCmdInfo info_domfsthaw[] = { + {.name = "help", + .data = N_("Thaw domain's mounted filesystems.") + }, + {.name = "desc", + .data = N_("Thaw domain's mounted filesystems.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_domfsthaw[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "mountpoints", + .type = VSH_OT_DATA, + .help = N_("comma separated list of mountpoints to be thawed") + }, + {.name = NULL} +}; +static bool +cmdDomFSThaw(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + int ret = -1; + unsigned int flags = 0; + const char *mountpoints_string = NULL; + char **list = NULL; /* tokenized mountpoints_string */ + int nmountpoints = 0; + size_t i; + + ignore_value(vshCommandOptString(cmd, "mountpoints", &mountpoints_string)); + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (mountpoints_string) { + if ((nmountpoints = vshStringToArray(mountpoints_string, &list)) < 0) + goto cleanup; + } + + ret = virDomainFSThaw(dom, (const char **)list, nmountpoints, flags); + if (ret < 0) { + vshError(ctl, _("Unable to thaw filesystems")); + goto cleanup; + } + + vshPrint(ctl, _("Thawed %d filesystem(s)\n"), ret); + + cleanup: + for (i = 0; i < nmountpoints; i++) + VIR_FREE(list[i]); + VIR_FREE(list); + virDomainFree(dom); + return ret >= 0; +} + const vshCmdDef domManagementCmds[] = { {.name = "attach-device", .handler = cmdAttachDevice, @@ -11538,6 +11656,18 @@ const vshCmdDef domManagementCmds[] = { .info = info_domdisplay, .flags = 0 }, + {.name = "domfsfreeze", + .handler = cmdDomFSFreeze, + .opts = opts_domfsfreeze, + .info = info_domfsfreeze, + .flags = 0 + }, + {.name = "domfsthaw", + .handler = cmdDomFSThaw, + .opts = opts_domfsthaw, + .info = info_domfsthaw, + .flags = 0 + }, {.name = "domfstrim", .handler = cmdDomFSTrim, .opts = opts_domfstrim, diff --git a/tools/virsh.pod b/tools/virsh.pod index abd2e93..89c58be 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -941,6 +941,29 @@ Output a URI which can be used to connect to the graphical display of the domain via VNC, SPICE or RDP. If I<--include-password> is specified, the SPICE channel password will be included in the URI. +=item B<domfsfreeze> I<domain> [I<--mountpoints> B<mountpoints>] + +Freeze mounted filesystems within a running domain to prepare for consistent +snapshots. + +The I<--mountpoints> option takes a parameter B<mountpoints>, which is a comma +separated list of mountpoint paths of filesystems to be frozen. If this is not +specified, every mounted filesystem is frozen. + +Note that B<snapshot-create> command has a I<--quiesce> option to freeze +and thaw the filesystems automatically to keep snapshots consistent. +B<domfsfreeze> command is only needed when a user wants to utilize the +native snapshot features of storage devices not supported by libvirt. + +=item B<domfsthaw> I<domain> [I<--mountpoints> B<mountpoints>] + +Thaw mounted filesystems within a running domain, which have been frozen by +domfsfreeze command. + +The I<--mountpoints> option takes a parameter B<mountpoints>, which is a comma +separated list of mountpoint paths of filesystems to be thawed. If it is not +specified, every mounted filesystem is thawed. + =item B<domfstrim> I<domain> [I<--minimum> B<bytes>] [I<--mountpoint mountPoint>]
On Tue, Apr 29, 2014 at 08:04:24PM -0400, Tomoki Sekiyama wrote:
These are exposed under domfsfreeze command and domfsthaw command.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> --- tools/virsh-domain.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 23 +++++++++ 2 files changed, 153 insertions(+)
+static const vshCmdOptDef opts_domfsfreeze[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "mountpoints", + .type = VSH_OT_DATA, + .help = N_("comma separated list of mountpoints to be frozen") + },
By using a command separated list, you prevent freezing of mount points whose directoy name contains a comma. I'd think it'd be better to just allow multiple instances of the arg eg virsh fsfreeze --mount <path1> --mount <path2> <guest>
+static const vshCmdOptDef opts_domfsthaw[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "mountpoints", + .type = VSH_OT_DATA, + .help = N_("comma separated list of mountpoints to be thawed") + },
Same comment here. Regards, 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 :|
On 4/30/14 11:30 , "Daniel P. Berrange" <berrange@redhat.com> wrote:
On Tue, Apr 29, 2014 at 08:04:24PM -0400, Tomoki Sekiyama wrote:
These are exposed under domfsfreeze command and domfsthaw command.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> --- tools/virsh-domain.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 23 +++++++++ 2 files changed, 153 insertions(+)
+static const vshCmdOptDef opts_domfsfreeze[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "mountpoints", + .type = VSH_OT_DATA, + .help = N_("comma separated list of mountpoints to be frozen") + },
By using a command separated list, you prevent freezing of mount points whose directoy name contains a comma.
Virsh parser treats ",," as an escaped character for a comma, so you can specify '/path,to/mnt' by '--mountpoints /path,,to/mnt'. And ...
I'd think it'd be better to just allow multiple instances of the arg eg
virsh fsfreeze --mount <path1> --mount <path2> <guest>
the virsh option parser rejects repeated options by error: option --mount already seen so I chose comma separated list, which is also used in the "undefine --storage" option that takes a list of targets or source paths. Do we need to extend the parser to enable the repeated options? Regards, Tomoki Sekiyama
On 04/30/2014 10:52 AM, Tomoki Sekiyama wrote:
I'd think it'd be better to just allow multiple instances of the arg eg
virsh fsfreeze --mount <path1> --mount <path2> <guest>
the virsh option parser rejects repeated options by
error: option --mount already seen
so I chose comma separated list, which is also used in the "undefine --storage" option that takes a list of targets or source paths.
Do we need to extend the parser to enable the repeated options?
The parser already accepts repeated options (well, precisely ONE repeated option), by making that option be last in the command description and giving it .type = VSH_OT_ARGV. For comparison, see the echo command in virsh.c or the send-key command in virsh-domain.c. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 4/30/14 15:44 , "Eric Blake" <eblake@redhat.com> wrote:
On 04/30/2014 10:52 AM, Tomoki Sekiyama wrote:
I'd think it'd be better to just allow multiple instances of the arg eg
virsh fsfreeze --mount <path1> --mount <path2> <guest>
the virsh option parser rejects repeated options by
error: option --mount already seen
so I chose comma separated list, which is also used in the "undefine --storage" option that takes a list of targets or source paths.
Do we need to extend the parser to enable the repeated options?
The parser already accepts repeated options (well, precisely ONE repeated option), by making that option be last in the command description and giving it .type = VSH_OT_ARGV. For comparison, see the echo command in virsh.c or the send-key command in virsh-domain.c.
Oh, I'd overlooked the type. Thank you for the references. I'll update this with VSH_OT_ARGV. Regards, Tomoki Sekiyama
On Wed, Apr 30, 2014 at 01:44:07PM -0600, Eric Blake wrote:
On 04/30/2014 10:52 AM, Tomoki Sekiyama wrote:
I'd think it'd be better to just allow multiple instances of the arg eg
virsh fsfreeze --mount <path1> --mount <path2> <guest>
the virsh option parser rejects repeated options by
error: option --mount already seen
so I chose comma separated list, which is also used in the "undefine --storage" option that takes a list of targets or source paths.
Do we need to extend the parser to enable the repeated options?
The parser already accepts repeated options (well, precisely ONE repeated option), by making that option be last in the command description and giving it .type = VSH_OT_ARGV. For comparison, see the echo command in virsh.c or the send-key command in virsh-domain.c.
That seems rather uneccessarily strict to me. I think it is entirely valid to allow any --fooo option to be marked as allowing repeats, not require it to be the final non-option argument. Regards, 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 :|
On 05/01/2014 03:38 AM, Daniel P. Berrange wrote:
The parser already accepts repeated options (well, precisely ONE repeated option), by making that option be last in the command description and giving it .type = VSH_OT_ARGV. For comparison, see the echo command in virsh.c or the send-key command in virsh-domain.c.
That seems rather uneccessarily strict to me. I think it is entirely valid to allow any --fooo option to be marked as allowing repeats, not require it to be the final non-option argument.
The repeated option is not required to be last in the user's command line, only that it is declared last in the C code. However, using a repeated option in anything other than the last position makes for more work for the end user. As an example, these two commands are equivalent: $ virsh echo --shell hello\! world 'hello!' world $ virsh echo --string hello\! --string world --shell 'hello!' world Supporting more than one option to be repeated might be possible (you'd have to patch the option parser to allow it), but using that gets tricky (you now HAVE to supply the option name for practically all repetitions of the option; at most one of the repeated options can take advantage of positional encoding for omitting the option name). But this particular example doesn't strike me as a case where we need multiple repeated options - multiple mountpoints is really all the more this command needs, and that works just fine with the existing framework, without needing to invent support for a command with two repeated options. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
With this patch, virDomainFSFreeze will pass the mountpoints argument to qemu guest agent. For example, virDomainFSFreeze(dom, {"/mnt/vol1", "/mnt/vol2"}, 2, 0) will issue qemu guest agent command: {"execute":"guest-fsfreeze-freeze", "arguments":{"mountpoints": ["/mnt/vol1","/mnt/vol2"]}} Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> --- src/qemu/qemu_agent.c | 47 +++++++++++++++++++++++++++++++++++++++++++---- src/qemu/qemu_agent.h | 3 ++- src/qemu/qemu_driver.c | 13 ++++++------- tests/qemuagenttest.c | 8 +++++--- 4 files changed, 56 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 4082331..57c7cc5 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1235,6 +1235,32 @@ qemuAgentMakeCommand(const char *cmdname, return NULL; } +static virJSONValuePtr +qemuAgentMakeStringsArray(const char **strings, unsigned int len) +{ + size_t i; + virJSONValuePtr ret = virJSONValueNewArray(), str; + + if (!ret) + return NULL; + + for (i = 0; i < len; i++) { + str = virJSONValueNewString(strings[i]); + if (!str) + goto error; + + if (virJSONValueArrayAppend(ret, str) < 0) { + virJSONValueFree(str); + goto error; + } + } + return ret; + + error: + virJSONValueFree(ret); + return NULL; +} + void qemuAgentNotifyEvent(qemuAgentPtr mon, qemuAgentEvent event) { @@ -1287,21 +1313,34 @@ int qemuAgentShutdown(qemuAgentPtr mon, /* * qemuAgentFSFreeze: * @mon: Agent + * @mountpoints: Array of mountpoint paths to be frozen, or NULL for all + * @nmountpoints: Number of mountpoints to be frozen, or 0 for all * * Issue guest-fsfreeze-freeze command to guest agent, - * which freezes all mounted file systems and returns + * which freezes file systems mounted on specified mountpoints + * (or all file systems when @mountpoints is NULL), 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 qemuAgentFSFreeze(qemuAgentPtr mon, const char **mountpoints, + unsigned int nmountpoints) { int ret = -1; - virJSONValuePtr cmd; + virJSONValuePtr cmd, arg; virJSONValuePtr reply = NULL; - cmd = qemuAgentMakeCommand("guest-fsfreeze-freeze", NULL); + if (mountpoints && nmountpoints) { + arg = qemuAgentMakeStringsArray(mountpoints, nmountpoints); + if (!arg) + return -1; + + cmd = qemuAgentMakeCommand("guest-fsfreeze-freeze", + "a:mountpoints", arg, NULL); + } else { + cmd = qemuAgentMakeCommand("guest-fsfreeze-freeze", NULL); + } if (!cmd) return -1; diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 5fbacdb..58531d5 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -70,7 +70,8 @@ typedef enum { int qemuAgentShutdown(qemuAgentPtr mon, qemuAgentShutdownMode mode); -int qemuAgentFSFreeze(qemuAgentPtr mon); +int qemuAgentFSFreeze(qemuAgentPtr mon, + const char **mountpoints, unsigned int nmountpoints); int qemuAgentFSThaw(qemuAgentPtr mon); int qemuAgentSuspend(qemuAgentPtr mon, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6bfd508..ae2d561 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12065,7 +12065,9 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, * returned, FSThaw should be called revert the quiesced status. */ static int qemuDomainSnapshotFSFreeze(virQEMUDriverPtr driver, - virDomainObjPtr vm) + virDomainObjPtr vm, + const char **mountpoints, + unsigned int nmountpoints) { qemuDomainObjPrivatePtr priv = vm->privateData; virQEMUDriverConfigPtr cfg; @@ -12091,7 +12093,7 @@ qemuDomainSnapshotFSFreeze(virQEMUDriverPtr driver, virObjectUnref(cfg); qemuDomainObjEnterAgent(vm); - freezed = qemuAgentFSFreeze(priv->agent); + freezed = qemuAgentFSFreeze(priv->agent, mountpoints, nmountpoints); qemuDomainObjExitAgent(vm); return freezed < 0 ? -2 : freezed; } @@ -13139,7 +13141,7 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, * The command will fail if the guest is paused or the guest agent * is not running, or is already quiesced. */ if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) { - int freeze = qemuDomainSnapshotFSFreeze(driver, vm); + int freeze = qemuDomainSnapshotFSFreeze(driver, vm, NULL, 0); if (freeze < 0) { /* the helper reported the error */ if (freeze == -2) @@ -16540,9 +16542,6 @@ qemuDomainFSFreeze(virDomainPtr dom, virCheckFlags(0, -1); - if (mountpoints || nmountpoints) - VIR_INFO("mountpoints option is not supported and ignored for now"); - if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; @@ -16558,7 +16557,7 @@ qemuDomainFSFreeze(virDomainPtr dom, goto endjob; } - ret = qemuDomainSnapshotFSFreeze(driver, vm); + ret = qemuDomainSnapshotFSFreeze(driver, vm, mountpoints, nmountpoints); if (ret == -2) { qemuDomainSnapshotFSThaw(driver, vm, false); ret = -1; diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 1131d98..be207e8 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -36,6 +36,7 @@ testQemuAgentFSFreeze(const void *data) { virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt); + const char *mountpoints[] = {"/fs1", "/fs2", "/fs3", "/fs4", "/fs5"}; int ret = -1; if (!test) @@ -55,7 +56,8 @@ testQemuAgentFSFreeze(const void *data) "{ \"return\" : 7 }") < 0) goto cleanup; - if ((ret = qemuAgentFSFreeze(qemuMonitorTestGetAgent(test))) < 0) + if ((ret = qemuAgentFSFreeze(qemuMonitorTestGetAgent(test), + mountpoints, 5)) < 0) goto cleanup; if (ret != 5) { @@ -64,7 +66,7 @@ testQemuAgentFSFreeze(const void *data) goto cleanup; } - if ((ret = qemuAgentFSFreeze(qemuMonitorTestGetAgent(test))) < 0) + if ((ret = qemuAgentFSFreeze(qemuMonitorTestGetAgent(test), NULL, 0)) < 0) goto cleanup; if (ret != 7) { @@ -547,7 +549,7 @@ testQemuAgentTimeout(const void *data) NULL, NULL) < 0) goto cleanup; - if (qemuAgentFSFreeze(qemuMonitorTestGetAgent(test)) != -1) { + if (qemuAgentFSFreeze(qemuMonitorTestGetAgent(test), NULL, 0) != -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "agent command should have failed"); goto cleanup;
On Tue, Apr 29, 2014 at 08:04:31PM -0400, Tomoki Sekiyama wrote:
With this patch, virDomainFSFreeze will pass the mountpoints argument to qemu guest agent. For example,
virDomainFSFreeze(dom, {"/mnt/vol1", "/mnt/vol2"}, 2, 0)
will issue qemu guest agent command:
{"execute":"guest-fsfreeze-freeze", "arguments":{"mountpoints": ["/mnt/vol1","/mnt/vol2"]}}
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> --- src/qemu/qemu_agent.c | 47 +++++++++++++++++++++++++++++++++++++++++++---- src/qemu/qemu_agent.h | 3 ++- src/qemu/qemu_driver.c | 13 ++++++------- tests/qemuagenttest.c | 8 +++++--- 4 files changed, 56 insertions(+), 15 deletions(-)
Hmm, seems QEMU guest agent silently ignores the mountpoints argument in the Win32 impl which rather sucks. We should ask QEMU to give us a way to query whether it is supported or not, so we can send a proper error message back to the app ACK, not a problem with your patch though. Regards, 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 :|
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Tomoki Sekiyama