Re: [Xen-devel] [PATCH: Lazy Restore] xl restore: Add a new "-l" lazy switch.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-05-29 at 00:34 +0530, Dushyant Behl wrote:
> @@ -1470,6 +1470,15 @@ int xc_domain_restore(xc_interface *xch, int io_fd, 
> uint32_t dom,
>  
>      DPRINTF("%s: starting restore of new domid %u", __func__, dom);
>  
> +    if ( lazy ) {

Coding style for libxc (see toplevel CODING_STYLE doc) has { on the next
line under the if.

Eventually you will need to target this work on top of the migration v2
work being done by Andrew Cooper, see
http://lists.xen.org/archives/html/xen-devel/2014-04/msg01377.html

You might find it easier to start on top of those patches anyway -- they
are a far cleaner base to work from.

> +        DPRINTF("xc: lazy switch enabled for restore\n");
> +
> +        if ( !hvm ) {
> +            PERROR("xc: lazy restore called for non HVM guest");
> +            return 1;
> +        }
> +    }
> +
>      pagebuf_init(&pagebuf);
>      memset(&tailbuf, 0, sizeof(tailbuf));
>      tailbuf.ishvm = hvm;
> diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h
> index 1f216cd..269275a 100644
> --- a/tools/libxc/xenguest.h
> +++ b/tools/libxc/xenguest.h
> @@ -126,7 +126,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, 
> uint32_t dom,
>                        unsigned long *console_mfn, domid_t console_domid,
>                        unsigned int hvm, unsigned int pae, int superpages,
>                        int no_incr_generationid, int checkpointed_stream,
> -                      unsigned long *vm_generationid_addr,
> +                      int lazy, unsigned long *vm_generationid_addr,
>                        struct restore_callbacks *callbacks);
>  /**
>   * xc_domain_restore writes a file to disk that contains the device
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index d015cf4..f599280 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -1348,8 +1348,8 @@ static void domain_create_cb(libxl__egc *egc,
>                               int rc, uint32_t domid);
>  
>  static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config,
> -                            uint32_t *domid,
> -                            int restore_fd, int checkpointed_stream,
> +                            uint32_t *domid, int restore_fd,
> +                            const libxl_domain_restore_params 
> *restore_params,
>                              const libxl_asyncop_how *ao_how,
>                              const libxl_asyncprogress_how *aop_console_how)
>  {
> @@ -1361,10 +1361,14 @@ static int do_domain_create(libxl_ctx *ctx, 
> libxl_domain_config *d_config,
>      cdcs->dcs.guest_config = d_config;
>      cdcs->dcs.restore_fd = restore_fd;
>      cdcs->dcs.callback = domain_create_cb;
> -    cdcs->dcs.checkpointed_stream = checkpointed_stream;
>      libxl__ao_progress_gethow(&cdcs->dcs.aop_console_how, aop_console_how);
>      cdcs->domid_out = domid;
>  
> +    if (restore_params) {
> +        cdcs->dcs.checkpointed_stream = restore_params->checkpointed_stream;

If !restore_params then cdcs->dcs.checkpointed_stream is no longer
explicitly initialised. Have you confirmed that it is zeroed somewhere?
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 52f1aa9..6c8355c 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -282,6 +282,7 @@ libxl_domain_create_info = Struct("domain_create_info",[
>  
>  libxl_domain_restore_params = Struct("domain_restore_params", [
>      ("checkpointed_stream", integer),
> +    ("lazy", integer),

Changes to the libxl interface require a LIBXL_HAVE #define in libxl.h,
see the comment regarding API compatibility in that file.

> @@ -3957,6 +3966,8 @@ int main_restore(int argc, char **argv)
>      case 'A':
>          vnc = vncautopass = 1;
>          break;
> +    case 'l':
> +        lazy = 1;

missing "break;"

> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> index 4279b9f..0405c6e 100644
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
> @@ -167,6 +167,7 @@ struct cmd_spec cmd_table[] = {
>        "Restore a domain from a saved state",
>        "[options] [<ConfigFile>] <CheckpointFile>",
>        "-h                       Print this help.\n"
> +      "-l                       Enable lazy restore.\n"

As Konrad says this also needs documentation, at least in the xl man
page.

Ian.


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