[libvirt] [PATCH 3/6] qemu: Use virDomainEventState helpers

[Cole Robinson]

On 01/13/2011 08:16 AM, Daniel P. Berrange wrote:
> On Mon, Jan 10, 2011 at 03:28:57PM -0500, Cole Robinson wrote:
>> v2:
>>     Drop libvirt_private.syms changes
>>
>> Signed-off-by: Cole Robinson <crobinso at redhat.com>
>> ---
>>  src/qemu/qemu_conf.h   |    6 +---
>>  src/qemu/qemu_driver.c |   80 +++++++++++++++--------------------------------
>>  2 files changed, 27 insertions(+), 59 deletions(-)
>>
>> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
>> index 5a5748b..2d878b5 100644
>> --- a/src/qemu/qemu_conf.h
>> +++ b/src/qemu/qemu_conf.h
>> @@ -107,11 +107,7 @@ struct qemud_driver {
>>  
>>      virCapsPtr caps;
>>  
>> -    /* An array of callbacks */
>> -    virDomainEventCallbackListPtr domainEventCallbacks;
>> -    virDomainEventQueuePtr domainEventQueue;
>> -    int domainEventTimer;
>> -    int domainEventDispatching;
>> +    virDomainEventStatePtr domainEventState;
>>  
>>      char *securityDriverName;
>>      virSecurityManagerPtr securityManager;
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 9eb9cd5..5acf1d9 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -1185,15 +1185,17 @@ qemudStartup(int privileged) {
>>      if (virDomainObjListInit(&qemu_driver->domains) < 0)
>>          goto out_of_memory;
>>  
>> -    /* Init callback list */
>> -    if (VIR_ALLOC(qemu_driver->domainEventCallbacks) < 0)
>> -        goto out_of_memory;
>> -    if (!(qemu_driver->domainEventQueue = virDomainEventQueueNew()))
>> -        goto out_of_memory;
>> -
>> -    if ((qemu_driver->domainEventTimer =
>> -         virEventAddTimeout(-1, qemuDomainEventFlush, qemu_driver, NULL)) < 0)
>> +    /* Init domain events */
>> +    qemu_driver->domainEventState = virDomainEventStateNew(qemuDomainEventFlush,
>> +                                                           qemu_driver,
>> +                                                           NULL);
>> +    if (!qemu_driver->domainEventState)
>>          goto error;
>> +    if (!qemu_driver->domainEventState->timer < 0) {
>> +        qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                        _("could not initialize domain event timer"));
>> +        goto error;
>> +    }
> 
> I'm curious about this ->timer < 0 check here. I'd kind of expect
> the 'domainEventState' struct to be opaque, and only accessible
> to the helper APIs you added as virDomainEventStateXXX. Could the
> virDomainEventStateNew() function simply return NULL if it was
> unable to initilize the timer, or is there a need for this to
> be treated as a non-fatal scenario ?
> 

For QEMU it is fatal, since the remote driver should always be
registering event handlers. But for the 'test' driver that isn't true.

> If it needs to be treated as non-fatal, could we pass in a bool
> flag to virDomainEventStateNew()   'bool requireTimer' so that
> it can enforce this itself rather than making the callers check

Sounds good.

Thanks,
Cole