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

[Hongyang Yang <yanghy@cn.fujitsu.com>]

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@cn.fujitsu.com>
>
> 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.