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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-03-25 at 14:45 +0000, Rob Hoes wrote:

> +static int domain_wait_event(libxl_ctx *ctx, int domid, libxl_event 
> **event_r)
> +{
> +     int ret;
> +     for (;;) {
> +             ret = libxl_event_wait(ctx, event_r, LIBXL_EVENTMASK_ALL, 0,0);
> +             if (ret) {
> +                     return ret;
> +             }
> +             if ((*event_r)->domid != domid) {
> +                     char *evstr = libxl_event_to_json(CTX, *event_r);
> +                     free(evstr);

Create/allocate the json and immediately free it? (left over debug
perhaps?)

> +                     libxl_event_free(CTX, *event_r);
> +                     continue;
> +             }
> +             return ret;
> +     }
> +}
[...]
> +value stub_xl_domain_create_restore(value ctx, value domain_config, value 
> restore_fd)
> +{
> +     CAMLparam2(ctx, domain_config);
> +     int ret;
> +     libxl_domain_config c_dconfig;
> +     uint32_t c_domid;
> +
> +     ret = domain_config_val(CTX, &c_dconfig, domain_config);
> +     if (ret != 0)
> +             failwith_xl(ret, "domain_create_restore");
> +
> +     ret = libxl_domain_create_restore(CTX, &c_dconfig, &c_domid, 
> Int_val(restore_fd), NULL, NULL);
> +     if (ret != 0)
> +             failwith_xl(ret, "domain_create_restore");
> +
> +     libxl_domain_config_dispose(&c_dconfig);
> +
> +     CAMLreturn(Val_int(c_domid));
> +}
> +
> +value stub_libxl_domain_wait_shutdown(value ctx, value domid)
> +{
> +     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) {
> +             fprintf(stderr,"wait for death failed (evgen, rc=%d)\n",ret);
> +             exit(-1);
> +     }
> +
> +     for (;;) {
> +             ret = domain_wait_event(CTX, Int_val(domid), &event);
> +             if (ret)
> +                     failwith_xl(ret, "domain_shutdown");

This exits asynchronously, which leaves the domain death event enabled.
Depending on what your exception handler does this may not be what you
want?

This case has only just occurred to me, so there may be other instances
in earlier patches...

> +
> +             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);
> +     libxl_evdisable_domain_death(CTX, deathw);
> +
> +     CAMLreturn(Val_unit);
> +}
> +
> +value stub_libxl_domain_shutdown(value ctx, value domid)
> +{
> +     CAMLparam2(ctx, domid);
> +     int ret;
> +
> +     ret = libxl_domain_shutdown(CTX, Int_val(domid));
> +
> +     if (ret != 0)
> +             failwith_xl(ret, "domain_shutdown");
> +
> +     CAMLreturn(Val_unit);
> +}

This and the rest look pretty mechanical, I just skimmed it...

Ian.


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