Re: [Xen-devel] [PATCH 04/19] libxl: domain save: API changes for asynchrony

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-06-08 at 18:34 +0100, Ian Jackson wrote:
> Change the internal and external APIs for domain save (suspend) to be
> capable of asynchronous operation.  The implementation remains
> synchronous.  The interfaces surrounding device model saving are still
> synchronous.
> 
> Public API changes:
> 
>  * libxl_domain_save takes an ao_how.
> 
>  * libxl_domain_remus_start takes an ao_how.  If the
>    libxl_domain_remus_info is NULL, we abort rather than returning an
>    error.
> 
>  * The `suspend_callback' function passed to libxl_domain_save is
>    never called by the existing implementation in libxl.  Abolish it.

Furthermore xl never passes one in either.
[...]

> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

A few minor comments below, but otherwise looks good to me.

[...]
> 
> +static void remus_crashed_cb(libxl__egc *egc,
> +                             libxl__domain_suspend_state *dss, int rc)

I'm not sure "crashed" is quite right here, it's finished for whatever
reason which may not necessarily be a crash (going forward it should
rarely be a crash, I think). It's "stopped" or "done" or something.

> [...]

> +int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
> +                         const libxl_asyncop_how *ao_how)
> +{
> +    AO_CREATE(ctx, domid, ao_how);
> +    int rc;
> +
>      libxl_domain_type type = libxl__domain_type(gc, domid);
> -    int live = info != NULL && info->flags & XL_SUSPEND_LIVE;
> -    int debug = info != NULL && info->flags & XL_SUSPEND_DEBUG;
> -    int rc = 0;
> +    if (type < 0) {
> +        LOG(ERROR,"domain %"PRIu32": unable to determine domain type", 
> domid);

libxl__domain_type now logs for you.

> +        rc = ERROR_FAIL;
> +        goto out_err;
> +    }
> 
> -    rc = libxl__domain_suspend_common(gc, domid, fd, type, live, debug,
> -                                      /* No Remus */ NULL);
> +    libxl__domain_suspend_state *dss;
> +    GCNEW(dss);
> 
> -    if (!rc && type == LIBXL_DOMAIN_TYPE_HVM)
> -        rc = libxl__domain_save_device_model(gc, domid, fd);
> -    GC_FREE;
> -    return rc;
> +    dss->ao = ao;
> +    dss->callback = domain_suspend_cb;
> +
> +    dss->domid = domid;
> +    dss->fd = fd;
> +    dss->type = type;
> +    dss->live = flags & LIBXL_SUSPEND_LIVE;
> +    dss->debug = flags & LIBXL_SUSPEND_DEBUG;
> +
> +    libxl__domain_suspend(egc, dss);
> +    return AO_INPROGRESS;
> +
> + out_err:
> +    return AO_ABORT(rc);
>  }
> 
>  int libxl_domain_pause(libxl_ctx *ctx, uint32_t domid)
[...]
> @@ -1903,10 +1915,27 @@ struct libxl__domain_create_state {
> 
>  /*----- Domain suspend (save) functions -----*/
> 
> -_hidden int libxl__domain_suspend_common(libxl__gc *gc, uint32_t domid, int 
> fd,
> -                                         libxl_domain_type type,
> -                                         int live, int debug,
> -                                         const libxl_domain_remus_info 
> *r_info);
> +/* calls callback when done */

Which callback? dss->callback I guess.

> +_hidden void libxl__domain_suspend(libxl__egc *egc,
> +                                   libxl__domain_suspend_state *dss);
> +
> +
> +/* calls libxl__xc_domain_suspend_done when done */
> +_hidden void libxl__xc_domain_save(libxl__egc*, libxl__domain_suspend_state*,
> +                                   unsigned long vm_generationid_addr);
> +/* If rc==0 then retval is the return value from xc_domain_save
> + * and errnoval is the errno value it provided.
> + * If rc!=0, retval and errnoval are undefined. */
> +_hidden void libxl__xc_domain_save_done(libxl__egc*,
> +                                        libxl__domain_suspend_state*,
> +                                        int rc, int retval, int errnoval);
> +
> +_hidden int libxl__domain_suspend_common_callback(void *data);
> +_hidden int libxl__domain_suspend_common_switch_qemu_logdirty
> +                               (int domid, unsigned int enable, void *data);
> +_hidden int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
> +        uint32_t *len, void *data);
> +
> 
>  /* calls libxl__xc_domain_restore_done when done */
>  _hidden void libxl__xc_domain_restore(libxl__egc *egc,



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