Re: [libvirt] [PATCHv2 2/5] remote: Implement bulk domain stats APIs in the remote driver
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
Attachments:
- signature.asc (application/pgp-signature — 539 bytes)