[libvirt] [PATCH v2 3/5] event: introduce new event for cputune

[Pavel Hrdina]

On 09/12/2014 09:46 PM, John Ferlan wrote:
>
>
> On 09/10/2014 08:08 AM, Pavel Hrdina wrote:
>> This new event will use typedParameters to expose what has been actually
>> updated and the reason is that we can in the future extend the cputune.
>> With typedParameters we don't have to worry about creating some sort of
>> v2 of cputune event if there will be new features implemented for cputune.
>>
>> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
>> ---
>>   daemon/remote.c              | 45 ++++++++++++++++++++++++
>>   include/libvirt/libvirt.h.in | 19 ++++++++++
>>   src/conf/domain_event.c      | 84 ++++++++++++++++++++++++++++++++++++++++++++
>>   src/conf/domain_event.h      |  9 +++++
>>   src/libvirt_private.syms     |  2 ++
>>   src/remote/remote_driver.c   | 41 +++++++++++++++++++++
>>   src/remote/remote_protocol.x | 14 +++++++-
>>   src/remote_protocol-structs  |  9 +++++
>>   tools/virsh-domain.c         | 33 +++++++++++++++++
>>   9 files changed, 255 insertions(+), 1 deletion(-)
>>
>
> Looks OK to me - there one detail below...
>

[..]

>>
>>   static void
>> +remoteDomainBuildEventCallbackCputune(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
>> +                                      virNetClientPtr client ATTRIBUTE_UNUSED,
>> +                                      void *evdata, void *opaque)
>> +{
>> +    virConnectPtr conn = opaque;
>> +    remote_domain_event_callback_cputune_msg *msg = evdata;
>> +    struct private_data *priv = conn->privateData;
>> +    virDomainPtr dom;
>> +    virTypedParameterPtr params = NULL;
>> +    int nparams = 0;
>> +    virObjectEventPtr event = NULL;
>> +
>> +    dom = get_nonnull_domain(conn, msg->dom);
>> +    if (!dom)
>> +        return;
>> +
>> +    if (remoteDeserializeTypedParameters(msg->params.params_val,
>> +                                         msg->params.params_len,
>> +                                         REMOTE_CPUMAPS_MAX,
>> +                                         &params, &nparams) < 0)
>> +        goto cleanup;
>> +
>> +    event = virDomainEventCputuneNewFromDom(dom, params, nparams);
>> +
>> +    remoteEventQueue(priv, event, msg->callbackID);
>> +
>> + cleanup:
>> +    virDomainFree(dom);
>
> Every other calling sequence as DomainFree followed by EventQueue - I
> have to believe there's a reason - although nothing pops right out.
>
> Like I said above in general ACK as I don't see anything obvious. It
> certainly seems easier for me to add iothreadpin events into my code.
>
> John
>

Yes that's true that every other event has virDomanFree followed by 
remoteEventQueue and the only reason I cant think of could be that 
virDomainFree is resetting last error, but I'm OK with changing the 
calling sequence.

Pavel