Re: [PATCH v3 2/9] tests/qtest: add support for callback to receive QMP events

[Juan Quintela]

Daniel P. Berrangé <berrange@redhat.com> wrote:
> Currently code must call one of the qtest_qmp_event* functions to
> fetch events. These are only usable if the immediate caller knows
> the particular event they want to capture, and are only interested
> in one specific event type. Adding ability to register an event
> callback lets the caller capture a range of events over any period
> of time.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

First of all, I *love* the idea of the patch, but ...

>  static GHookList abrt_hooks;
> @@ -703,8 +705,14 @@ QDict *qtest_qmp_receive(QTestState *s)
>          if (!qdict_get_try_str(response, "event")) {
>              return response;
>          }
> -        /* Stash the event for a later consumption */
> -        s->pending_events = g_list_append(s->pending_events, response);
> +
> +        if (s->eventCB) {
> +            s->eventCB(s, qdict_get_str(response, "event"),
> +                       response, s->eventData);
> +        } else {
> +            /* Stash the event for a later consumption */
> +            s->pending_events = g_list_append(s->pending_events, response);
> +        }
>      }

s->eventCB returns a bool that you are not using.  I think this part of
the code would be more usefule if:

        if (!s->eventCB || !s->eventCB(s, qdict_get_str(response, "event"),
                                       response, s->eventData)) {
            /* Stash the event for a later consumption */
            s->pending_events = g_list_append(s->pending_events, response);
        }

So if we are not handling the event, we put it on the pending_events
list.

What do you think?

Later, Juan.