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