On 08/26/2014 12:22 PM, Eric Blake wrote: > On 08/26/2014 08:14 AM, Peter Krempa wrote: >> Implement the remote driver support for shuffling the domain stats >> around. >> --- >> daemon/remote.c | 91 ++++++++++++++++++++++++++++++++++++++++++++ >> src/remote/remote_driver.c | 88 ++++++++++++++++++++++++++++++++++++++++++ >> src/remote/remote_protocol.x | 26 ++++++++++++- >> src/remote_protocol-structs | 23 +++++++++++ >> 4 files changed, 227 insertions(+), 1 deletion(-) > > Rearranging things in my review (man, I wish git format-patch -Ofilename > could be offloaded into git config by more than just an alias - it's > nice to have a filename that lists .x and .h to show before .c). > >> + >> + /** >> + * @generate: none >> + * @acl: domain:read >> + */ >> + REMOTE_PROC_DOMAIN_LIST_GET_STATS = 343 > > Merge conflicts; trivial to resolve :) Oops, missed a review comment. This is the wrong ACL. I think you want to have: @acl: connect:search_domains @aclfilter: domain:read which says that you can only call this API if you are also allowed to call virConnectListAllDomains (connect:search_domains), and that within this API, you will be filtering any domain that the user cannot normally obtain stats on via other means (for example, virDomainGetState and virDomainGetCPUStats both require domain:read - but they are called on a single domain, whereas this function is called on every domain and filtering out the domains that cannot be called individually). I didn't audit if ALL stats that you plan on querying all use domain:read, or if there are even more limitations to place on this API. Which means patch 3/5 is probably busted for not using a filter function. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list