Re: [Xen-devel] [PATCH v3-RESEND 24/28] libxl: ocaml: add VM lifecycle operations

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-10-21 at 14:32 +0100, Rob Hoes wrote:

> +static int domain_wait_event(libxl_ctx *ctx, int domid, libxl_event 
> **event_r)

Do you not have the infrastructure in place to bind this to ocaml and do
the next level of the call chain in ocaml instead of C?
> +{
> +     int ret;
> +     for (;;) {
> +             ret = libxl_event_wait(ctx, event_r, LIBXL_EVENTMASK_ALL, 0,0);
> +             if (ret) {
> +                     return ret;
> +             }
> +             if ((*event_r)->domid != domid) {
> +                     libxl_event_free(CTX, *event_r);
> +                     continue;
> +             }
> +             return ret;
> +     }
> +}
> +
> +value stub_libxl_domain_create_new(value ctx, value domain_config, value 
> async, value unit)
> +{
> [...]
> +     libxl_asyncop_how ao_how;
> +
> +     if (async != Val_none) {
> +             ao_how.callback = async_callback;
> +             ao_how.u.for_callback = (void *) Some_val(async);
> +     }
[...]
> +             async != Val_none ? &ao_how : NULL, NULL);

Lots of this patten. What about
#define OCAML_ASYNC_HOW \
        libxl_asyncop_how _ao_how; \
        libxl_asyncop_how *ao_how = NULL; \
        if (async != Val_none) \
                _ao_how.callback = ...
                _ao_how.u.for_callback = ...
                ao_how = &_ao_how

Then an unconditional ao_how at the point of use?

This relies on this file being compiled to allow mixed code and variable
definitions. We do that for libxl itself, not sure about here. Could be
enabled.

Or at least the initialisation of ao_how could be made a function.
Perhaps 
        static value Val_oahow(value aohow struct aohow *c_aohow)
        {
                if (aohow == Val_none) return NULL;

                c_aohow->etc 
                return c_aohow
        }

then 
        libxl_aohow aohow;

        libxl_do_foo(....
                Val_aohow(async, &aohow), ...);

> +value stub_libxl_domain_wait_shutdown(value ctx, value domid)
> +{

This is the thing I meant could you do in ocaml above...

> +     CAMLparam2(ctx, domid);
> +     int ret;
> +     libxl_event *event;
> +     libxl_evgen_domain_death *deathw;
> +     ret = libxl_evenable_domain_death(CTX, Int_val(domid), 0, &deathw);
> +     if (ret)
> +             failwith_xl(ret, "domain_wait_shutdown");
> +
> +     for (;;) {
> +             ret = domain_wait_event(CTX, Int_val(domid), &event);
> +             if (ret) {
> +                     libxl_evdisable_domain_death(CTX, deathw);
> +                     failwith_xl(ret, "domain_wait_shutdown");
> +             }
> +
> +             switch (event->type) {
> +             case LIBXL_EVENT_TYPE_DOMAIN_DEATH:
> +                     goto done;
> +             case LIBXL_EVENT_TYPE_DOMAIN_SHUTDOWN:
> +                     goto done;
> +             default:
> +                     break;
> +             }
> +             libxl_event_free(CTX, event);
> +     }
> +done:
> +     libxl_event_free(CTX, event);

I don't know, but this might be clearer with
        the_right-type type = event->type;
        lbixl_event_free(...)
before the switch, which then becomes switch(type)?

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel