Re: [Xen-devel] [PATCH v11 14/27] secondary vm suspend/resume/checkpoint code

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Changlong Xie writes ("[PATCH v11 14/27] secondary vm suspend/resume/checkpoint 
code"):
> From: Wen Congyang <wency@xxxxxxxxxxxxxx>
> 
> Secondary vm is running in colo mode. So we will do
> the following things again and again:

I don't propose to review this in detail.  Skimreading it, it looks
plausible.  I don't think a detailed review is needed.

I will review the changes to the core code.

> diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
> index 5d9f497..2bfed64 100644
> --- a/tools/libxc/xc_sr_common.h
> +++ b/tools/libxc/xc_sr_common.h
> @@ -184,10 +184,12 @@ struct xc_sr_context
>       * migration stream
>       * 0: Plain VM
>       * 1: Remus
> +     * 2: COLO
>       */
>      enum {
>          MIG_STREAM_NONE, /* plain stream */
>          MIG_STREAM_REMUS,
> +        MIG_STREAM_COLO,

I think this shows that the duplicated list (in the comment, above the
enum) is a mistake.  I would prefer it to be removed.

> +/* ===================== colo: common functions ===================== */
> +static void colo_enable_logdirty(libxl__colo_restore_state *crs, libxl__egc 
> *egc)

Here's another missing blank line.  This seems to be a general theme:
can you change this everywhere ?  Thanks.

> @@ -994,6 +1011,8 @@ static void domcreate_bootloader_done(libxl__egc *egc,
>      const int restore_fd = dcs->restore_fd;
>      libxl__domain_build_state *const state = &dcs->build_state;
>      const int checkpointed_stream = dcs->restore_params.checkpointed_stream;
> +    libxl__colo_restore_state *const crs = &dcs->crs;
> +    libxl_domain_build_info *const info = &d_config->b_info;
>  
>      if (rc) {
>          domcreate_rebuild_done(egc, dcs, rc);
> @@ -1022,6 +1041,13 @@ static void domcreate_bootloader_done(libxl__egc *egc,
>  
>      /* Restore */
>  
> +    /* COLO only supports HVM now */
> +    if (info->type != LIBXL_DOMAIN_TYPE_HVM &&
> +        checkpointed_stream == LIBXL_CHECKPOINTED_STREAM_COLO) {
> +        rc = ERROR_FAIL;
> +        goto out;

Please log something here, or it may be very mysterious.

> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 6307b71..48b4858 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -87,6 +87,8 @@
> @@ -3468,7 +3464,6 @@ libxl__stream_read_inuse(const libxl__stream_read_state 
> *stream)
>      return stream->running;
>  }
>  
> -
>  struct libxl__domain_create_state {
>      /* filled in by user */
>      libxl__ao *ao;
> @@ -3484,6 +3479,8 @@ struct libxl__domain_create_state {

Unintentional whitespace change.


Ian.

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