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
Attachments:
- signature.asc (application/pgp-signature — 539 bytes)