Re: [Xen-devel] [PATCH 15/15] libxl: New event generation API

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 5 Dec 2011, Ian Jackson wrote:
> +int libxl_evenable_domain_death(libxl_ctx *ctx, uint32_t domid,
> +                libxl_ev_user user, libxl_evgen_domain_death **evgen_out) {
> +    GC_INIT(ctx);
> +    libxl_evgen_domain_death *evg, *evg_search;
> +    int rc;
> +
> +    CTX_LOCK;
> +
> +    evg = malloc(sizeof(*evg));  if (!evg) { rc = ERROR_NOMEM; goto out; }
> +    memset(evg, 0, sizeof(*evg));
> +    evg->domid = domid;
> +    evg->user = user;
> +
> +    LIBXL_TAILQ_INSERT_SORTED(&ctx->death_list, entry, evg, evg_search, ,

is this a mistake?                                                        ^


> +void libxl__event_occurred(libxl__gc *gc, libxl_event *event) {
> +    if (CTX->event_hooks &&
> +        (CTX->event_hooks->event_occurs_mask & (1UL << event->type))) {
> +        /* libxl__free_all will call the callback, just before exit
> +         * from libxl.

Please extend this comment: "just before leaving libxl to go back to the
caller".  Also, even though libxl__free_all might be the right place to
call the callbacks, the name of the function (libxl__free_all) won't
completely reflect its behaviour anymore. Maybe we need to rename
libxl__free_all?


> +int libxl_event_check(libxl_ctx *ctx, libxl_event **event_r,
> +                      unsigned long typemask,
> +                      libxl_event_predicate *pred, void *pred_user) {
> +    GC_INIT(ctx);
> +    CTX_LOCK;
> +    int rc = event_check_unlocked(gc, event_r, typemask, pred, pred_user);
> +    CTX_UNLOCK;
> +    GC_FREE;
> +    return rc;
> +}

I think it is confusing to call event_check_*unlocked* from within
CTX_LOCK/CTX_UNLOCK.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel