[libvirt] [PATCH v5 2/4] event: introduce new event for tunable values

[Pavel Hrdina]

On 09/23/2014 11:50 PM, Eric Blake wrote:
> On 09/23/2014 03:26 PM, Pavel Hrdina wrote:
>
>>>> +
>>>> +    VIR_DEBUG("Relaying domain tunable event %s %d, callback %d",
>>>> +              dom->name, dom->id, callback->callbackID);
>>>> +
>>>
>>> Might also be nice to log "%p %n", params, nparams
>>
>> Yes, that would be probably nice, but since I've pushed this patch
>> already I can create a following patch with this small update?
>>
>
> Yes, a followup is fine.
>
>>>
>>>
>>>> +++ b/src/remote/remote_protocol.x
>>>> @@ -247,6 +247,9 @@ const REMOTE_NETWORK_DHCP_LEASES_MAX = 65536;
>>>>    /* Upper limit on count of parameters returned via bulk stats API */
>>>>    const REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX = 4096;
>>>>
>>>> +/* Upper limit of message size for tunable event. */
>>>> +const REMOTE_DOMAIN_EVENT_TUNABLE_MAX = 8388608;
>>>
>>> That feels excessive...
>>>
>>>> +
>>>>    /* UUID.  VIR_UUID_BUFLEN definition comes from libvirt.h */
>>>>    typedef opaque remote_uuid[VIR_UUID_BUFLEN];
>>>>
>>>> @@ -2990,6 +2993,12 @@ struct remote_domain_event_block_job_2_msg {
>>>>        int status;
>>>>    };
>>>>
>>>> +struct remote_domain_event_callback_tunable_msg {
>>>> +    int callbackID;
>>>> +    remote_nonnull_domain dom;
>>>> +    remote_typed_param params<REMOTE_DOMAIN_EVENT_TUNABLE_MAX>;
>>>
>>> ...each param in the array will occupy multiple bytes.  I think that
>>> something as low as 2048 for REMOTE_DOMAIN_EVENT_TUNABLE_MAX is still
>>> plenty (we don't have that many tunables yet); even if each tunable
>>> requires 64 bytes to transmit (mostly in the name of the parameter, but
>>> also in the type and value), that's still well under a megabyte limit of
>>> information passed on an instance of the event.
>>>
>>
>> Well, yes and no :). Let's say, that we will add in the future (and I'm
>> planning to do it) blkiotune where you can update at the same time all
>> of the tunables for all host's disks where all params for now will be
>> only VIR_TYPED_PARAM_STRING and that could consume a lot of memory. I
>> know that it will probably never be that much, but I wanted to be sure
>> that we will have enough space for all possible tunable events.
>
> Still, are you going to return 8 million separate strings?  Or just 8
> million bytes but still contained within 2000 strings?  Seriously, I
> think 2048 is a perfectly LARGE limit - there are not THAT many tunables
> per domain.  The params<LIMIT> is not the overall size of the command,
> but the number of parameters (each of which can be quite large if they
> are type string)
>

Sigh, I should not work that late, because I've misunderstood the
meaning of the "LIMIT". I'll post a new value with patch for the
debug message.

Thanks, Pavel