[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 0/5] Guest filesystem information API



On 11/19/2014 03:58 PM, John Ferlan wrote:
> 
> 
> On 11/17/2014 06:26 PM, Tomoki Sekiyama wrote:
>> Hi,
>>
>> This is v2 of patchset to add virDomainGetFSInfo API.
>>
>> * changes in v1->v2:
>>  -[all] removed redundant NULL element at the last of returned info array
>>  -[3/5] make error messages in qemu_agent.c consistent with other commands
>>  -[4/5] added a test case for 2 items in info->devAliases
>>  -[5/5] added a pod document for virsh domfsinfo command
>>  (v1: http://www.redhat.com/archives/libvir-list/2014-October/msg00001.html )
>>

> I reviewed the 'libvirt' specific changes - had a few comments and have
> made changes to my review branch as specified.  As long as you're OK
> with those changes I will get these pushed.

I think it may be worth a v3 if you like my suggestion on 1/5 of
providing a count argument for the length of the alias array, rather
than forcing the clients to crawl the array themselves to find that length.

> 
> I'm also hoping someone else (eblake?) can look at the remote_protocol.x
> changes to ensure they encompass everything they are supposed to.  Also
> that the usage of QEMU_JOB_QUERY not _MODIFY for the GetFSInfo seems
> more appropriate and is in line with the various remote_protocol.x
> settings (@acl/@generate stuff settings).

On my cursory glance, and reading Michal's review, yes, it looks like
that part was done right.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]