From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?windows-1252?Q?Roger_Pau_Monn=E9?= Subject: Re: [PATCH v3 09/32] libxl: switch HVM domain building to use xc_dom_* helpers Date: Tue, 28 Jul 2015 16:26:15 +0200 Message-ID: <55B79107.3030402@citrix.com> References: <1435923310-9019-1-git-send-email-roger.pau@citrix.com> <1435923310-9019-10-git-send-email-roger.pau@citrix.com> <20150728112239.GL5111@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZK5pn-0002Bg-PI for xen-devel@lists.xenproject.org; Tue, 28 Jul 2015 14:26:23 +0000 In-Reply-To: <20150728112239.GL5111@zion.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wei Liu Cc: xen-devel@lists.xenproject.org, Ian Jackson , Ian Campbell , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org El 28/07/15 a les 13.22, Wei Liu ha escrit: > 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 use= d in >> order to build PV guests. >> >> Signed-off-by: Roger Pau Monn=E9 >> Cc: Ian Jackson >> Cc: Stefano Stabellini >> Cc: Ian Campbell >> Cc: Wei Liu > = > Mostly looks good. Some nits below. Thanks for the review. >> --- >> 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 d= omid, >> 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 =3D libxl__gc_owner(gc); > = > No need to have this line. But ctx is needed... >> + uint64_t mem_kb; >> + int ret; >> + >> + if ( (ret =3D xc_dom_boot_xen_init(dom, ctx->xch, domid)) !=3D 0 ) { ^ here. >> + LOGE(ERROR, "xc_dom_boot_xen_init failed"); >> + goto out; >> + } >> +#ifdef GUEST_RAM_BASE >> + if ( (ret =3D xc_dom_rambase_init(dom, GUEST_RAM_BASE)) !=3D 0 ) { >> + LOGE(ERROR, "xc_dom_rambase failed"); >> + goto out; >> + } >> +#endif >> + if ( (ret =3D xc_dom_parse_image(dom)) !=3D 0 ) { >> + LOGE(ERROR, "xc_dom_parse_image failed"); >> + goto out; >> + } >> + if ( (ret =3D libxl__arch_domain_init_hw_description(gc, info, stat= e, dom)) !=3D 0 ) { >> + LOGE(ERROR, "libxl__arch_domain_init_hw_description failed"); >> + goto out; >> + } >> + >> + mem_kb =3D dom->container_type =3D=3D XC_DOM_HVM_CONTAINER ? >> + (info->max_memkb - info->video_memkb) : info->target_memkb; >> + if ( (ret =3D xc_dom_mem_init(dom, mem_kb / 1024)) !=3D 0 ) { >> + LOGE(ERROR, "xc_dom_mem_init failed"); >> + goto out; >> + } >> + if ( (ret =3D xc_dom_boot_mem_init(dom)) !=3D 0 ) { >> + LOGE(ERROR, "xc_dom_boot_mem_init failed"); >> + goto out; >> + } >> + if ( (ret =3D libxl__arch_domain_finalise_hw_description(gc, info, = dom)) !=3D 0 ) { >> + LOGE(ERROR, "libxl__arch_domain_finalise_hw_description failed"= ); >> + goto out; >> + } >> + if ( (ret =3D xc_dom_build_image(dom)) !=3D 0 ) { >> + LOGE(ERROR, "xc_dom_build_image failed"); >> + goto out; >> + } >> + if ( (ret =3D xc_dom_boot_image(dom)) !=3D 0 ) { >> + LOGE(ERROR, "xc_dom_boot_image failed"); >> + goto out; >> + } >> + if ( (ret =3D xc_dom_gnttab_init(dom)) !=3D 0 ) { >> + LOGE(ERROR, "xc_dom_gnttab_init failed"); >> + goto out; >> + } >> + >> +out: >> + return ret; > = > Please translate libxc error code to libxl error code. Done, I have however just done: return ret !=3D 0 ? ERROR_FAIL : 0; Not sure if we need/want a more complex error code translation here. >> +} >> + >> 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] =3D 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 =3D libxl__gc_owner(gc); >> - struct xc_hvm_build_args args =3D {}; >> - int ret, rc =3D ERROR_FAIL; >> - uint64_t mmio_start, lowmem_end, highmem_end; >> + int ret; > = > This should be rc. Right, we don't call any libxc functions directly any longer, so only rc should be used. Roger.