Re: [libvirt] [PATCH v6 1/6] Introduce virDomainFSFreeze() and virDomainFSThaw() public API

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(a)hds.com&gt; > 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