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

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

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.

> 
> 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