[libvirt] [PATCH v2] lib: Add note that bulk stats API queries may overrun RPC buffers

[Michal Privoznik]

On 05/23/2017 05:19 PM, Martin Kletzander wrote:
> On Tue, May 23, 2017 at 05:07:40PM +0200, Michal Privoznik wrote:
>> On 05/23/2017 04:35 PM, Martin Kletzander wrote:
>>> On Tue, May 23, 2017 at 04:23:30PM +0200, Peter Krempa wrote:
>>>> Hint that the users should limit the number of VMs queried in the bulk
>>>> stats API.
>>>> ---
>>>> v2:
>>>> - added a suggestion of the number of queried VMs (valid after bump to
>>>> 32M message)
>>>>
>>>> src/libvirt-domain.c | 8 ++++++++
>>>> 1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
>>>> index 310b91b37..aef061943 100644
>>>> --- a/src/libvirt-domain.c
>>>> +++ b/src/libvirt-domain.c
>>>> @@ -11315,6 +11315,10 @@ virConnectGetDomainCapabilities(virConnectPtr
>>>> conn,
>>>>  * VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF and/or
>>>>  * VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER for all other states.
>>>>  *
>>>> + * Note that this API is prone to exceeding maximum RPC message size
>>>> on hosts
>>>> + * with lots of VMs so it's suggested to use virDomainListGetStats
>>>> with a
>>>> + * reasonable list of VMs as the argument.
>>>> + *
>>>>  * Returns the count of returned statistics structures on success, -1
>>>> on error.
>>>>  * The requested data are returned in the @retStats parameter. The
>>>> returned
>>>>  * array should be freed by the caller. See
>>>> virDomainStatsRecordListFree.
>>>> @@ -11386,6 +11390,10 @@ virConnectGetAllDomainStats(virConnectPtr
>>>> conn,
>>>>  * Note that any of the domain list filtering flags in @flags may be
>>>> rejected
>>>>  * by this function.
>>>>  *
>>>> + * Note that this API is prone to exceeding maximum RPC if querying
>>>> too many VMs
>>>> + * with lots of statistics. It's suggested to query in batches of
>>>> 10VMs, which
>>>> + * should be good enough for VMs with 3000 disks + networks.
>>>> + *
>>>
>>> Coming to think about it... Why don't we just batch this ourselves under
>>> the hood and just return the merged result?
>>
>> Because:
>>
>> https://www.redhat.com/archives/libvir-list/2017-May/msg00088.html
>>
> 
> Not on the RPC level, the API would just be syntax sugar to
> virDomainListGetStats() if a flag was passed
> (e.g.
> VIR_DOMAIN_GET_ALL_STATS_I_DONT_REALLY_CARE_IF_THIS_IS_DONE_IN_ONE_LIBVIRT_CALL)

We can't guarantee atomicity of such call, so I don't see a reason for
us to implement it. On the other hand, it's us who knows the limits for
RPC, not users. So they might have some troubles writing it. But then
again, we would have some troubles too because we might be talking to an
older server with lower limits thus we'd need some guessing too.

Also, the flag would expose what we want to hide - RPC layer. It should
be no concern for users how we pass data around.

Michal