Re: [PATCH v9 04/25] libxc/migration: export read_record for common use

From: Wen Congyang <wency@cn.fujitsu.com>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Lars Kurth <lars.kurth@citrix.com>,
	Changlong Xie <xiecl.fnst@cn.fujitsu.com>,
	Wei Liu <wei.liu2@citrix.com>,
	Ian Campbell <ian.campbell@citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Jiang Yunhong <yunhong.jiang@intel.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	xen devel <xen-devel@lists.xen.org>,
	Dong Eddie <eddie.dong@intel.com>,
	Gui Jianfeng <guijianfeng@cn.fujitsu.com>,
	Shriram Rajagopalan <rshriram@cs.ubc.ca>,
	Yang Hongyang <hongyang.yang@easystack.cn>
Subject: Re: [PATCH v9 04/25] libxc/migration: export read_record for common use
Date: Wed, 27 Jan 2016 08:57:04 +0800	[thread overview]
Message-ID: <56A815E0.9030501@cn.fujitsu.com> (raw)
In-Reply-To: <20160126204527.GC27940@char.us.oracle.com>

On 01/27/2016 04:45 AM, Konrad Rzeszutek Wilk wrote:
> On Wed, Dec 30, 2015 at 10:37:34AM +0800, Wen Congyang wrote:
>> read_record() could be used by primary to read dirty bitmap
>> record sent by secondary under COLO.
>> When used by save side, we need to pass the backchannel fd
>> instead of ctx->fd to read_record(), so we added a fd param to
>> it.
> 
> Could you add:
> 
> No functional changes.

OK, will fix it in the next version.

Thanks
Wen Congyang

> 
>>
>> Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn>
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>  tools/libxc/xc_sr_common.c  | 49 +++++++++++++++++++++++++++++++++++
>>  tools/libxc/xc_sr_common.h  | 14 ++++++++++
>>  tools/libxc/xc_sr_restore.c | 63 +--------------------------------------------
>>  3 files changed, 64 insertions(+), 62 deletions(-)
>>
>> diff --git a/tools/libxc/xc_sr_common.c b/tools/libxc/xc_sr_common.c
>> index 8150140..42ee074 100644
>> --- a/tools/libxc/xc_sr_common.c
>> +++ b/tools/libxc/xc_sr_common.c
>> @@ -89,6 +89,55 @@ int write_split_record(struct xc_sr_context *ctx, struct xc_sr_record *rec,
>>      return -1;
>>  }
>>  
>> +int read_record(struct xc_sr_context *ctx, int fd, struct xc_sr_record *rec)
>> +{
>> +    xc_interface *xch = ctx->xch;
>> +    struct xc_sr_rhdr rhdr;
>> +    size_t datasz;
>> +
>> +    if ( read_exact(fd, &rhdr, sizeof(rhdr)) )
>> +    {
>> +        PERROR("Failed to read Record Header from stream");
>> +        return -1;
>> +    }
>> +    else if ( rhdr.length > REC_LENGTH_MAX )
>> +    {
>> +        ERROR("Record (0x%08x, %s) length %#x exceeds max (%#x)", rhdr.type,
>> +              rec_type_to_str(rhdr.type), rhdr.length, REC_LENGTH_MAX);
>> +        return -1;
>> +    }
>> +
>> +    datasz = ROUNDUP(rhdr.length, REC_ALIGN_ORDER);
>> +
>> +    if ( datasz )
>> +    {
>> +        rec->data = malloc(datasz);
>> +
>> +        if ( !rec->data )
>> +        {
>> +            ERROR("Unable to allocate %zu bytes for record data (0x%08x, %s)",
>> +                  datasz, rhdr.type, rec_type_to_str(rhdr.type));
>> +            return -1;
>> +        }
>> +
>> +        if ( read_exact(fd, rec->data, datasz) )
>> +        {
>> +            free(rec->data);
>> +            rec->data = NULL;
>> +            PERROR("Failed to read %zu bytes of data for record (0x%08x, %s)",
>> +                   datasz, rhdr.type, rec_type_to_str(rhdr.type));
>> +            return -1;
>> +        }
>> +    }
>> +    else
>> +        rec->data = NULL;
>> +
>> +    rec->type   = rhdr.type;
>> +    rec->length = rhdr.length;
>> +
>> +    return 0;
>> +};
>> +
>>  static void __attribute__((unused)) build_assertions(void)
>>  {
>>      XC_BUILD_BUG_ON(sizeof(struct xc_sr_ihdr) != 24);
>> diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
>> index bc99e9a..53d6129 100644
>> --- a/tools/libxc/xc_sr_common.h
>> +++ b/tools/libxc/xc_sr_common.h
>> @@ -370,6 +370,20 @@ static inline int write_record(struct xc_sr_context *ctx,
>>  }
>>  
>>  /*
>> + * Reads a record from the stream, and fills in the record structure.
>> + *
>> + * Returns 0 on success and non-0 on failure.
>> + *
>> + * On success, the records type and size shall be valid.
>> + * - If size is 0, data shall be NULL.
>> + * - If size is non-0, data shall be a buffer allocated by malloc() which must
>> + *   be passed to free() by the caller.
>> + *
>> + * On failure, the contents of the record structure are undefined.
>> + */
>> +int read_record(struct xc_sr_context *ctx, int fd, struct xc_sr_record *rec);
>> +
>> +/*
>>   * This would ideally be private in restore.c, but is needed by
>>   * x86_pv_localise_page() if we receive pagetables frames ahead of the
>>   * contents of the frames they point at.
>> diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
>> index d4dc501..e543be3 100644
>> --- a/tools/libxc/xc_sr_restore.c
>> +++ b/tools/libxc/xc_sr_restore.c
>> @@ -69,67 +69,6 @@ static int read_headers(struct xc_sr_context *ctx)
>>  }
>>  
>>  /*
>> - * Reads a record from the stream, and fills in the record structure.
>> - *
>> - * Returns 0 on success and non-0 on failure.
>> - *
>> - * On success, the records type and size shall be valid.
>> - * - If size is 0, data shall be NULL.
>> - * - If size is non-0, data shall be a buffer allocated by malloc() which must
>> - *   be passed to free() by the caller.
>> - *
>> - * On failure, the contents of the record structure are undefined.
>> - */
>> -static int read_record(struct xc_sr_context *ctx, struct xc_sr_record *rec)
>> -{
>> -    xc_interface *xch = ctx->xch;
>> -    struct xc_sr_rhdr rhdr;
>> -    size_t datasz;
>> -
>> -    if ( read_exact(ctx->fd, &rhdr, sizeof(rhdr)) )
>> -    {
>> -        PERROR("Failed to read Record Header from stream");
>> -        return -1;
>> -    }
>> -    else if ( rhdr.length > REC_LENGTH_MAX )
>> -    {
>> -        ERROR("Record (0x%08x, %s) length %#x exceeds max (%#x)", rhdr.type,
>> -              rec_type_to_str(rhdr.type), rhdr.length, REC_LENGTH_MAX);
>> -        return -1;
>> -    }
>> -
>> -    datasz = ROUNDUP(rhdr.length, REC_ALIGN_ORDER);
>> -
>> -    if ( datasz )
>> -    {
>> -        rec->data = malloc(datasz);
>> -
>> -        if ( !rec->data )
>> -        {
>> -            ERROR("Unable to allocate %zu bytes for record data (0x%08x, %s)",
>> -                  datasz, rhdr.type, rec_type_to_str(rhdr.type));
>> -            return -1;
>> -        }
>> -
>> -        if ( read_exact(ctx->fd, rec->data, datasz) )
>> -        {
>> -            free(rec->data);
>> -            rec->data = NULL;
>> -            PERROR("Failed to read %zu bytes of data for record (0x%08x, %s)",
>> -                   datasz, rhdr.type, rec_type_to_str(rhdr.type));
>> -            return -1;
>> -        }
>> -    }
>> -    else
>> -        rec->data = NULL;
>> -
>> -    rec->type   = rhdr.type;
>> -    rec->length = rhdr.length;
>> -
>> -    return 0;
>> -};
>> -
>> -/*
>>   * Is a pfn populated?
>>   */
>>  static bool pfn_is_populated(const struct xc_sr_context *ctx, xen_pfn_t pfn)
>> @@ -646,7 +585,7 @@ static int restore(struct xc_sr_context *ctx)
>>  
>>      do
>>      {
>> -        rc = read_record(ctx, &rec);
>> +        rc = read_record(ctx, ctx->fd, &rec);
>>          if ( rc )
>>          {
>>              if ( ctx->restore.buffer_all_records )
>> -- 
>> 2.5.0
>>
>>
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
> 
> 
> .
> 