[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] events: Don't fail on registering events for two different domains



On 27.06.2012 14:46, Daniel P. Berrange wrote:
> On Wed, Jun 27, 2012 at 02:44:02PM +0200, Michal Privoznik wrote:
>> On 27.06.2012 14:36, Eric Blake wrote:
>>> On 06/27/2012 06:12 AM, Michal Privoznik wrote:
>>>> virConnectDomainEventRegisterAny() takes a domain as an argument.
>>>> So it should be possible to register the same event (be it
>>>> VIR_DOMAIN_EVENT_ID_LIFECYCLE for example) for two different domains.
>>>> That is, we need to take domain into account when searching for
>>>> duplicate event being already registered.
>>>> ---
>>>>  src/conf/domain_event.c |    6 +++++-
>>>>  1 files changed, 5 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
>>>> index 4ecc413..3cfd940 100644
>>>> --- a/src/conf/domain_event.c
>>>> +++ b/src/conf/domain_event.c
>>>> @@ -363,7 +363,11 @@ virDomainEventCallbackListAddID(virConnectPtr conn,
>>>>      for (i = 0 ; i < cbList->count ; i++) {
>>>>          if (cbList->callbacks[i]->cb == VIR_DOMAIN_EVENT_CALLBACK(callback) &&
>>>>              cbList->callbacks[i]->eventID == eventID &&
>>>> -            cbList->callbacks[i]->conn == conn) {
>>>> +            cbList->callbacks[i]->conn == conn &&
>>>> +            ((dom && cbList->callbacks[i]->dom &&
>>>> +              memcmp(cbList->callbacks[i]->dom->uuid,
>>>> +                     dom->uuid, VIR_UUID_BUFLEN) == 0) ||
>>>> +             (!dom && !cbList->callbacks[i]->dom))) {
>>>
>>> This misses the case of registering a catchall against NULL domain then
>>> attempting to re-register the same event against a specific domain
>>> (which one of the two would fire?).  It also misses the case of
>>> registering a domain-specific handler, then attempting to broaden things
>>> into a global handler.
>>
>> Yes, but that's intentional. In both cases both events are fired.
>>
>>>
>>> I think the idea of double registration makes sense (in particular, if I
>>> have a per-domain callback, but then want to switch to global, I would
>>> rather have a window with both handlers at once than a window with no
>>> handler at all), but I have not tested whether we actually handle it by
>>> firing both the global and domain-specific callback.
>>
>> I've tested it and it works.
> 
> How ?  The remote driver does not ever pass the virDomainPtr arg
> over the wire, and it restricts itself to 1 single callback per
> event type. Only the client side drivers (test, esx, virtualbox)
> could possibly work, but not KVM, LXC, Xen, UML
> 
> Daniel
> 

I am attaching the reproducer. My output:

zippy bart ~/work/tmp $ gcc repro.c -o repro -lvirt -ggdb3 && LD_LIBRARY_PATH="/home/mprivozn/work/libvirt/libvirt.git/src/.libs/" ./repro qemu:///system
myDomainEventCallback2 EVENT: Domain f16(-1) Defined Updated o:cb1
myDomainEventCallback2 EVENT: Domain f17(-1) Defined Updated o:cb1
myDomainEventCallback2 EVENT: Domain f17(-1) Defined Updated o:cb2

So we can see cb1 and cb2 firing up at once (cb1 is registered against NULL, cb2 against f17 domain).

Michal
#include <libvirt/libvirt.h>
#include <libvirt/virterror.h>
#include <stdio.h>
#include <stdlib.h>
#include <signal.h>
#include <string.h>

int run = 1;

static void stop(int sig)
{
    printf("Exiting on signal %d\n", sig);
    run = 0;
}

const char *eventToString(int event) {
    const char *ret = "";
    switch ((virDomainEventType) event) {
        case VIR_DOMAIN_EVENT_DEFINED:
            ret ="Defined";
            break;
        case VIR_DOMAIN_EVENT_UNDEFINED:
            ret ="Undefined";
            break;
        case VIR_DOMAIN_EVENT_STARTED:
            ret ="Started";
            break;
        case VIR_DOMAIN_EVENT_SUSPENDED:
            ret ="Suspended";
            break;
        case VIR_DOMAIN_EVENT_RESUMED:
            ret ="Resumed";
            break;
        case VIR_DOMAIN_EVENT_STOPPED:
            ret ="Stopped";
            break;
        case VIR_DOMAIN_EVENT_SHUTDOWN:
            ret = "Shutdown";
            break;
    }
    return ret;
}

static const char *eventDetailToString(int event, int detail) {
    const char *ret = "";
    switch ((virDomainEventType) event) {
        case VIR_DOMAIN_EVENT_DEFINED:
            if (detail == VIR_DOMAIN_EVENT_DEFINED_ADDED)
                ret = "Added";
            else if (detail == VIR_DOMAIN_EVENT_DEFINED_UPDATED)
                ret = "Updated";
            break;
        case VIR_DOMAIN_EVENT_UNDEFINED:
            if (detail == VIR_DOMAIN_EVENT_UNDEFINED_REMOVED)
                ret = "Removed";
            break;
        case VIR_DOMAIN_EVENT_STARTED:
            switch ((virDomainEventStartedDetailType) detail) {
            case VIR_DOMAIN_EVENT_STARTED_BOOTED:
                ret = "Booted";
                break;
            case VIR_DOMAIN_EVENT_STARTED_MIGRATED:
                ret = "Migrated";
                break;
            case VIR_DOMAIN_EVENT_STARTED_RESTORED:
                ret = "Restored";
                break;
            case VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT:
                ret = "Snapshot";
                break;
            case VIR_DOMAIN_EVENT_STARTED_WAKEUP:
                ret = "Event wakeup";
                break;
            }
            break;
        case VIR_DOMAIN_EVENT_SUSPENDED:
            switch ((virDomainEventSuspendedDetailType) detail) {
            case VIR_DOMAIN_EVENT_SUSPENDED_PAUSED:
                ret = "Paused";
                break;
            case VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED:
                ret = "Migrated";
                break;
            case VIR_DOMAIN_EVENT_SUSPENDED_IOERROR:
                ret = "I/O Error";
                break;
            case VIR_DOMAIN_EVENT_SUSPENDED_WATCHDOG:
                ret = "Watchdog";
                break;
            case VIR_DOMAIN_EVENT_SUSPENDED_RESTORED:
                ret = "Restored";
                break;
            case VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT:
                ret = "Snapshot";
                break;
            }
            break;
        case VIR_DOMAIN_EVENT_RESUMED:
            switch ((virDomainEventResumedDetailType) detail) {
            case VIR_DOMAIN_EVENT_RESUMED_UNPAUSED:
                ret = "Unpaused";
                break;
            case VIR_DOMAIN_EVENT_RESUMED_MIGRATED:
                ret = "Migrated";
                break;
            case VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT:
                ret = "Snapshot";
                break;
            }
            break;
        case VIR_DOMAIN_EVENT_STOPPED:
            switch ((virDomainEventStoppedDetailType) detail) {
            case VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN:
                ret = "Shutdown";
                break;
            case VIR_DOMAIN_EVENT_STOPPED_DESTROYED:
                ret = "Destroyed";
                break;
            case VIR_DOMAIN_EVENT_STOPPED_CRASHED:
                ret = "Crashed";
                break;
            case VIR_DOMAIN_EVENT_STOPPED_MIGRATED:
                ret = "Migrated";
                break;
            case VIR_DOMAIN_EVENT_STOPPED_SAVED:
                ret = "Failed";
                break;
            case VIR_DOMAIN_EVENT_STOPPED_FAILED:
                ret = "Failed";
                break;
            case VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT:
                ret = "Snapshot";
                break;
            }
            break;
        case VIR_DOMAIN_EVENT_SHUTDOWN:
            switch ((virDomainEventShutdownDetailType) detail) {
            case VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED:
                ret = "Finished";
                break;
            }
            break;
    }
    return ret;
}
static int myDomainEventCallback2(virConnectPtr conn,
                                  virDomainPtr dom,
                                  int event,
                                  int detail,
                                  void *opaque)
{
    printf("%s EVENT: Domain %s(%d) %s %s o:%s\n", __func__, virDomainGetName(dom),
           virDomainGetID(dom), eventToString(event),
           eventDetailToString(event, detail), (char *)opaque);
    return 0;
}

static void myFreeFunc(void *opaque)
{
    char *str = opaque;
    printf("%s: Freeing [%s]\n", __func__, str);
    free(str);
}

int main(int argc, char *argv[]) {
    virConnectPtr conn = NULL;
    virDomainPtr dom1 = NULL, dom2 = NULL;
    int callback1ret = -1, callback2ret = -1;
    int ret = -1;
    struct sigaction action_stop;

    memset(&action_stop, 0, sizeof(action_stop));
    action_stop.sa_handler = stop;
    sigaction(SIGTERM, &action_stop, NULL);
    sigaction(SIGINT, &action_stop, NULL);

    virEventRegisterDefaultImpl();

    conn = virConnectOpenAuth(argc > 1 ? argv[1] : NULL, virConnectAuthPtrDefault, 0);

    if (!conn) {
       fprintf(stderr, "conn\n");
       return -1;
    }

    dom1 = virDomainLookupByName(conn, argc > 2 ? argv[2] : "f16");
    dom2 = virDomainLookupByName(conn, argc > 3 ? argv[3] : "f17");

    if (!dom1 || !dom2) {
        fprintf(stderr, "dom\n");
        goto cleanup;
    }

    callback1ret = virConnectDomainEventRegisterAny(conn,
                                                    NULL,
                                                    VIR_DOMAIN_EVENT_ID_LIFECYCLE,
                                                    VIR_DOMAIN_EVENT_CALLBACK(myDomainEventCallback2),
                                                    strdup("cb1"), myFreeFunc);
    callback2ret = virConnectDomainEventRegisterAny(conn,
                                                    dom2,
                                                    VIR_DOMAIN_EVENT_ID_LIFECYCLE,
                                                    VIR_DOMAIN_EVENT_CALLBACK(myDomainEventCallback2),
                                                    strdup("cb2"), myFreeFunc);

    if (callback1ret < 0) {
        fprintf(stderr, ":( cb1\n");
        goto cleanup;
    }

    if (callback2ret < 0) {
        fprintf(stderr, ":( cb2\n");
        goto cleanup;
    }

    if (virConnectSetKeepAlive(conn, 5, 3) < 0) {
        fprintf(stderr, "ka\n");
        goto cleanup;
    }

    while (run && virConnectIsAlive(conn) == 1) {
        if (virEventRunDefaultImpl() < 0) {
            fprintf(stderr, "event\n");
        }
    }

    ret = 0;

cleanup:
    if (callback1ret >= 0)
        virConnectDomainEventDeregisterAny(conn, callback1ret);
    if (callback2ret >= 0)
        virConnectDomainEventDeregisterAny(conn, callback2ret);

    if (dom1)
        virDomainFree(dom1);
    if (dom2)
        virDomainFree(dom2);

    if (conn)
        virConnectClose(conn);

    return ret;
}

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]