Re: [PATCH v5 21/28] libxl: info: Display build_id of the hypervisor using XEN_VERSION_build_id

[Wei Liu <wei.liu2@citrix.com>]

On Fri, Mar 25, 2016 at 09:25:22AM -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Mar 24, 2016 at 04:00:33PM -0400, Konrad Rzeszutek Wilk wrote:
> > If the hypervisor is built with we will display it.
> > 
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> Hey Wei,
> 
> It has you Ack, but I think when I carried over the change (it used
> to be its own function with switch) I messed up the Style:
> 
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index 6c3ec40..310a7f3 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -5277,8 +5278,24 @@ const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
> >  
> >      info->virt_start = val;
> >  
> > -    (void)libxl__xc_version_wrapper(gc, XEN_VERSION_commandline, buf,
> > -                                    info->pagesize, &info->commandline);
> > +    if (libxl__xc_version_wrapper(gc, XEN_VERSION_commandline, buf,
> > +                                  info->pagesize, &info->commandline) < 0)
> > +        goto out;
> > +
> > +    r = xc_version(ctx->xch, XEN_VERSION_build_id, buf, info->pagesize);
> > +    if (r < 0)
> > +    {
> > +        info->build_id = libxl__strdup(NOGC, "");
> > +    }
> > +    else if (r > 0)
> > +    {
> > +        unsigned int i;
> > +
> > +        info->build_id = libxl__zalloc(NOGC, (r * 2) + 1);
> > +
> > +        for (i = 0; i < r; i++)
> > +            snprintf(&info->build_id[i * 2], 3, "%02hhx", buf[i]);
> > +    }
> >   out:
> >      GC_FREE;
> >      return info;
> 
> So I fixed it up to be:
> 
> From bc4ed9d93162325342a37122fcab7223fcd61430 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Fri, 18 Mar 2016 14:56:13 -0400
> Subject: [PATCH] libxl: info: Display build_id of the hypervisor using
>  XEN_VERSION_build_id
> 
> If the hypervisor is built with we will display it.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 

I skim-read it. Looks fine:

Acked-by: Wei Liu <wei.liu2@citrix.com>

> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> 
> v2: Include HAVE_*, use libxl_zalloc, s/rc/ret/
> v3: Retry with different size if 1020 is not enough.
> v4: Use VERSION_OP subops instead of the XENVER_ subops
> v5: Change it per Wei's review. s/VERSION_OP/VERSION/
>     And actually use the proper Style!
> ---
>  tools/libxl/libxl.c         | 18 ++++++++++++++++--
>  tools/libxl/libxl.h         |  6 ++++++
>  tools/libxl/libxl_types.idl |  1 +
>  tools/libxl/xl_cmdimpl.c    |  1 +
>  4 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 704e7b4..dea5d25 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -5233,6 +5233,7 @@ const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
>      GC_INIT(ctx);
>      char *buf;
>      xen_version_op_val_t val = 0;
> +    int r;
>      libxl_version_info *info = &ctx->version_info;
>  
>      if (info->xen_version_extra != NULL)
> @@ -5275,8 +5276,21 @@ const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
>  
>      info->virt_start = val;
>  
> -    (void)libxl__xc_version_wrapper(gc, XEN_VERSION_commandline, buf,
> -                                    info->pagesize, &info->commandline);
> +    if (libxl__xc_version_wrapper(gc, XEN_VERSION_commandline, buf,
> +                                  info->pagesize, &info->commandline) < 0)
> +        goto out;
> +
> +    r = xc_version(ctx->xch, XEN_VERSION_build_id, buf, info->pagesize);
> +    if (r < 0) {
> +        info->build_id = libxl__strdup(NOGC, "");
> +    } else if (r > 0) {
> +        unsigned int i;
> +
> +        info->build_id = libxl__zalloc(NOGC, (r * 2) + 1);
> +
> +        for (i = 0; i < r; i++)
> +            snprintf(&info->build_id[i * 2], 3, "%02hhx", buf[i]);
> +    }
>   out:
>      GC_FREE;
>      return info;
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index f61bc4b..5baffdf 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -230,6 +230,12 @@
>  #define LIBXL_HAVE_APIC_ASSIST 1
>  
>  /*
> + * LIBXL_HAVE_BUILD_ID means that libxl_version_info has the extra
> + * field for the hypervisor build_id.
> + */
> +#define LIBXL_HAVE_BUILD_ID 1
> +
> +/*
>   * libxl ABI compatibility
>   *
>   * The only guarantee which libxl makes regarding ABI compatibility
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 59b183c..e3a5707 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -363,6 +363,7 @@ libxl_version_info = Struct("version_info", [
>      ("virt_start",        uint64),
>      ("pagesize",          integer),
>      ("commandline",       string),
> +    ("build_id",          string),
>      ], dir=DIR_OUT)
>  
>  libxl_domain_create_info = Struct("domain_create_info",[
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index a3610fc..23da95e 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -5861,6 +5861,7 @@ static void output_xeninfo(void)
>      printf("cc_compile_by          : %s\n", info->compile_by);
>      printf("cc_compile_domain      : %s\n", info->compile_domain);
>      printf("cc_compile_date        : %s\n", info->compile_date);
> +    printf("build_id               : %s\n", info->build_id);
>  
>      return;
>  }
> -- 
> 2.5.0
> 

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