Re: [Xen-devel] [PATCH Remus v1 4/8] tools/libxc: split read/handle qemu info

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/05/15 07:37, Yang Hongyang wrote:
> Split read/handle qemu info. The receiving of qemu info
> should be done while we receive the migration stream,
> handle_qemu will be called when the stream complete.
> Otherwise, it will break Remus because read_record()
> won't read qemu info and stream_complete will be called
> at failover.
>
> Signed-off-by: Yang Hongyang <yanghy@xxxxxxxxxxxxxx>

I presume this is because Remus sends multiple qemu records in the stream?

I will be fixing the qemu record layer violation as part of libxl
migration v2, at which point all the XG_LIBXL_HVM_COMPAT code shall
disappear.

As all this code appears to live inside XG_LIBXL_HVM_COMPAT, I am
willing to trust that it DoesTheRightThing, if you have confirmed that
plain HVM migration with migration v2 and XG_LIBXL_HVM_COMPAT continues
to work (I am slightly suspicious of the 3rd hunk in this regard).

~Andrew

> ---
>  tools/libxc/xc_sr_common.h          |  5 +++++
>  tools/libxc/xc_sr_restore.c         | 12 ++++++++++++
>  tools/libxc/xc_sr_restore_x86_hvm.c | 28 +++++++++++++++++++++++++---
>  3 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
> index ef42412..6f099b8 100644
> --- a/tools/libxc/xc_sr_common.h
> +++ b/tools/libxc/xc_sr_common.h
> @@ -279,6 +279,11 @@ struct xc_sr_context
>                      /* HVM context blob. */
>                      void *context;
>                      size_t contextsz;
> +
> +#ifdef XG_LIBXL_HVM_COMPAT
> +                    uint32_t qlen;
> +                    void *qbuf;
> +#endif
>                  } restore;
>              };
>          } x86_hvm;
> diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
> index 53bd674..8022c3d 100644
> --- a/tools/libxc/xc_sr_restore.c
> +++ b/tools/libxc/xc_sr_restore.c
> @@ -510,6 +510,9 @@ static int process_record(struct xc_sr_context *ctx, 
> struct xc_sr_record *rec)
>      return rc;
>  }
>  
> +#ifdef XG_LIBXL_HVM_COMPAT
> +extern int read_qemu(struct xc_sr_context *ctx);
> +#endif
>  /*
>   * Restore a domain.
>   */
> @@ -546,6 +549,15 @@ static int restore(struct xc_sr_context *ctx)
>  
>      } while ( rec.type != REC_TYPE_END );
>  
> +#ifdef XG_LIBXL_HVM_COMPAT
> +    if ( ctx->dominfo.hvm )
> +    {
> +        rc = read_qemu(ctx);
> +        if ( rc )
> +            goto err;
> +    }
> +#endif
> +
>      rc = ctx->restore.ops.stream_complete(ctx);
>      if ( rc )
>          goto err;
> diff --git a/tools/libxc/xc_sr_restore_x86_hvm.c 
> b/tools/libxc/xc_sr_restore_x86_hvm.c
> index 6e9b318..6f5af0e 100644
> --- a/tools/libxc/xc_sr_restore_x86_hvm.c
> +++ b/tools/libxc/xc_sr_restore_x86_hvm.c
> @@ -94,14 +94,14 @@ static int handle_hvm_params(struct xc_sr_context *ctx,
>  }
>  
>  #ifdef XG_LIBXL_HVM_COMPAT
> -static int handle_qemu(struct xc_sr_context *ctx)
> +int read_qemu(struct xc_sr_context *ctx);
> +int read_qemu(struct xc_sr_context *ctx)
>  {
>      xc_interface *xch = ctx->xch;
> -    char qemusig[21], path[256];
> +    char qemusig[21];
>      uint32_t qlen;
>      void *qbuf = NULL;
>      int rc = -1;
> -    FILE *fp = NULL;
>  
>      if ( read_exact(ctx->fd, qemusig, sizeof(qemusig)) )
>      {
> @@ -137,6 +137,28 @@ static int handle_qemu(struct xc_sr_context *ctx)
>          goto out;
>      }
>  
> +    /* With Remus, this could be read many times */
> +    if ( ctx->x86_hvm.restore.qbuf )
> +        free(ctx->x86_hvm.restore.qbuf);
> +    ctx->x86_hvm.restore.qbuf = qbuf;
> +    ctx->x86_hvm.restore.qlen = qlen;
> +    rc = 0;
> +
> +out:
> +    if (rc)
> +        free(qbuf);
> +    return rc;
> +}
> +
> +static int handle_qemu(struct xc_sr_context *ctx)
> +{
> +    xc_interface *xch = ctx->xch;
> +    char path[256];
> +    uint32_t qlen = ctx->x86_hvm.restore.qlen;
> +    void *qbuf = ctx->x86_hvm.restore.qbuf;
> +    int rc = -1;
> +    FILE *fp = NULL;
> +
>      sprintf(path, XC_DEVICE_MODEL_RESTORE_FILE".%u", ctx->domid);
>      fp = fopen(path, "wb");
>      if ( !fp )


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