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@xxxxxxxxxx>
>> ---
>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list