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

[Tomoki Sekiyama]

Hi, thank you for the review.

On 4/30/14 11:26 , "Daniel P. Berrange" <berrange at 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 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"

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