Re: [Xen-devel] [PATCH v3 09/32] libxl: switch HVM domain building to use xc_dom_* helpers

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Jul 03, 2015 at 01:34:47PM +0200, Roger Pau Monne wrote:
> Now that we have all the code in place HVM domain building in libxl can be
> switched to use the xc_dom_* family of functions, just like they are used in
> order to build PV guests.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>

Mostly looks good. Some nits below.

> ---
>  tools/libxl/libxl_dom.c      | 224 
> +++++++++++++++++++++++++------------------
>  tools/libxl/libxl_internal.h |   2 +-
>  tools/libxl/libxl_vnuma.c    |  12 ++-
>  3 files changed, 139 insertions(+), 99 deletions(-)
> 
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 2bae277..480b7e7 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -609,6 +609,64 @@ static int set_vnuma_info(libxl__gc *gc, uint32_t domid,
>      return rc;
>  }
>  
> +static int libxl__build_dom(libxl__gc *gc, uint32_t domid,
> +             libxl_domain_build_info *info, libxl__domain_build_state *state,
> +             struct xc_dom_image *dom)
> +{
> +    libxl_ctx *ctx = libxl__gc_owner(gc);

No need to have this line.

> +    uint64_t mem_kb;
> +    int ret;
> +
> +    if ( (ret = xc_dom_boot_xen_init(dom, ctx->xch, domid)) != 0 ) {
> +        LOGE(ERROR, "xc_dom_boot_xen_init failed");
> +        goto out;
> +    }
> +#ifdef GUEST_RAM_BASE
> +    if ( (ret = xc_dom_rambase_init(dom, GUEST_RAM_BASE)) != 0 ) {
> +        LOGE(ERROR, "xc_dom_rambase failed");
> +        goto out;
> +    }
> +#endif
> +    if ( (ret = xc_dom_parse_image(dom)) != 0 ) {
> +        LOGE(ERROR, "xc_dom_parse_image failed");
> +        goto out;
> +    }
> +    if ( (ret = libxl__arch_domain_init_hw_description(gc, info, state, 
> dom)) != 0 ) {
> +        LOGE(ERROR, "libxl__arch_domain_init_hw_description failed");
> +        goto out;
> +    }
> +
> +    mem_kb = dom->container_type == XC_DOM_HVM_CONTAINER ?
> +             (info->max_memkb - info->video_memkb) : info->target_memkb;
> +    if ( (ret = xc_dom_mem_init(dom, mem_kb / 1024)) != 0 ) {
> +        LOGE(ERROR, "xc_dom_mem_init failed");
> +        goto out;
> +    }
> +    if ( (ret = xc_dom_boot_mem_init(dom)) != 0 ) {
> +        LOGE(ERROR, "xc_dom_boot_mem_init failed");
> +        goto out;
> +    }
> +    if ( (ret = libxl__arch_domain_finalise_hw_description(gc, info, dom)) 
> != 0 ) {
> +        LOGE(ERROR, "libxl__arch_domain_finalise_hw_description failed");
> +        goto out;
> +    }
> +    if ( (ret = xc_dom_build_image(dom)) != 0 ) {
> +        LOGE(ERROR, "xc_dom_build_image failed");
> +        goto out;
> +    }
> +    if ( (ret = xc_dom_boot_image(dom)) != 0 ) {
> +        LOGE(ERROR, "xc_dom_boot_image failed");
> +        goto out;
> +    }
> +    if ( (ret = xc_dom_gnttab_init(dom)) != 0 ) {
> +        LOGE(ERROR, "xc_dom_gnttab_init failed");
> +        goto out;
> +    }
> +
> +out:
> +    return ret;

Please translate libxc error code to libxl error code.

> +}
> +
>  int libxl__build_pv(libxl__gc *gc, uint32_t domid,
>               libxl_domain_build_info *info, libxl__domain_build_state *state)
>  {
> @@ -699,48 +757,9 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
>              dom->vnode_to_pnode[i] = info->vnuma_nodes[i].pnode;
>      }
>  
[...]
>      }
>  
> @@ -919,50 +943,60 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
>                libxl__domain_build_state *state)
>  {
>      libxl_ctx *ctx = libxl__gc_owner(gc);
> -    struct xc_hvm_build_args args = {};
> -    int ret, rc = ERROR_FAIL;
> -    uint64_t mmio_start, lowmem_end, highmem_end;
> +    int ret;

This should be rc.

Wei.

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