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

[Hongyang Yang <yanghy@xxxxxxxxxxxxxx>]

On 05/07/2015 06:48 PM, Andrew Cooper wrote:
> 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.
ye, this patch should disappear with your fix :)

> 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).
I tested both pv&hvm, normal migration are all work as it is.

> ~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 )
> .
--
Thanks,
Yang.

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