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

[Daniel P. Berrange]

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 at 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 :|