Re: [Xen-devel] [PATCH v2 01/03] HVM firmware passthrough libxl support

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2013-02-01 at 20:15 +0000, Ross Philipson wrote:
> Switch libxl to use the new xc_hvm_build() libxc API.
> 
> Signed-off-by: Ross Philipson <ross.philipson@xxxxxxxxxx>
> 
> diff -r 27778b4099ba tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c   Fri Jan 25 15:04:11 2013 +0000
> +++ b/tools/libxl/libxl_dom.c   Fri Jan 25 13:37:55 2013 -0500
> @@ -542,17 +542,24 @@ int libxl__build_hvm(libxl__gc *gc, uint
>                libxl__domain_build_state *state)
>  {
>      libxl_ctx *ctx = libxl__gc_owner(gc);
> +    struct xc_hvm_build_args args = {};
>      int ret, rc = ERROR_FAIL;
>      const char *firmware = libxl__domain_firmware(gc, info);
>  
>      if (!firmware)
>          goto out;
> -    ret = xc_hvm_build_target_mem(
> -        ctx->xch,
> -        domid,
> -        (info->max_memkb - info->video_memkb) / 1024,
> -        (info->target_memkb - info->video_memkb) / 1024,
> -        firmware);
> +
> +    memset(&args, 0, sizeof(struct xc_hvm_build_args));
> +    /* The memory size names are misleading. The params are in Mb then
> +     * multiplied by 1 Kb.

What's misleading about max_memkb? It contains kb doesn't it? Likewise
the others.

The actual code in this patch looks good to me. Assuming I've remembered
the precedence of cast vs << correctly.

>  This was then divided off when calling
> +     * the old xc_hvm_build_target_mem() which then turned them to bytes.
> +     * Do all this in one step here...
> +     */
> +    args.mem_size = (uint64_t)(info->max_memkb - info->video_memkb) << 10;
> +    args.mem_target = (uint64_t)(info->target_memkb - info->video_memkb) << 
> 10;
> +    args.image_file_name = firmware;
> +
> +    ret = xc_hvm_build(ctx->xch, domid, &args);
>      if (ret) {
>          LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, ret, "hvm building 
> failed");
>          goto out; 


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