Hi, thank you for the review.
On 4/30/14 11:26 , "Daniel P. Berrange" <berrange@xxxxxxxxxx> 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@xxxxxxx>
>
>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list