Re: [Xen-devel] [V1 PATCH 3/5] PVH xen tools: libxl changes to create a PVH guest.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-09-04 at 18:32 -0700, Mukesh Rathor wrote:
> Signed-off-by: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
> ---
>  tools/libxl/libxl_create.c   |   12 ++++++++++++
>  tools/libxl/libxl_dom.c      |    5 ++++-
>  tools/libxl/libxl_internal.h |    1 +
>  tools/libxl/libxl_types.idl  |    1 +
>  tools/libxl/libxl_x86.c      |    4 +++-
>  tools/libxl/xl_cmdimpl.c     |    1 +

Please also modify libxl.h to add a suitable LIBXL_HAVE_xxx #define as
described in the big comment about API compatibility.

>  6 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 0c32d0b..0dfc4b7 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -33,6 +33,9 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
>      if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) {
>          libxl_defbool_setdefault(&c_info->hap, true);
>          libxl_defbool_setdefault(&c_info->oos, true);
> +    } else {
> +        libxl_defbool_setdefault(&c_info->pvh, false);
> +        libxl_defbool_setdefault(&c_info->hap, false);

Make this:
           libxl_defbool_setdefault(&c_info->hap, 
libxl_defbool_val(c_info->pvh);
so that HAP defaults on if PVH is on.

>      }
>  
>      libxl_defbool_setdefault(&c_info->run_hotplug_scripts, true);
> @@ -346,6 +349,8 @@ int libxl__domain_build(libxl__gc *gc,
>  
>          break;
>      case LIBXL_DOMAIN_TYPE_PV:
> +        state->pvh_enabled = libxl_defbool_val(d_config->c_info.pvh);
> +
>          ret = libxl__build_pv(gc, domid, info, state);
>          if (ret)
>              goto out;
> @@ -405,6 +410,13 @@ int libxl__domain_make(libxl__gc *gc, 
> libxl_domain_create_info *info,
>          flags |= XEN_DOMCTL_CDF_hvm_guest;
>          flags |= libxl_defbool_val(info->hap) ? XEN_DOMCTL_CDF_hap : 0;
>          flags |= libxl_defbool_val(info->oos) ? 0 : XEN_DOMCTL_CDF_oos_off;
> +    } else if ( libxl_defbool_val(info->pvh) ) {
> +        if ( !libxl_defbool_val(info->hap) ) {
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "HAP must be on for PVH");

Please use LOG(ERROR, "...") in new code.

> +            rc = ERROR_INVAL;
> +            goto out;
> +        }
> +        flags |= XEN_DOMCTL_CDF_hap;
>      }
>      *domid = -1;
>  
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 6e2252a..d84313f 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -338,6 +338,8 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
>          return ERROR_FAIL;
>      }
>  
> +    dom->pvh_enabled = state->pvh_enabled;
> +
>      LOG(DEBUG, "pv kernel mapped %d path %s\n", state->pv_kernel.mapped, 
> state->pv_kernel.path);
>      if (state->pv_kernel.mapped) {
>          ret = xc_dom_kernel_mem(dom,
> @@ -400,7 +402,8 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
>          LOGE(ERROR, "xc_dom_boot_image failed");
>          goto out;
>      }
> -    if ( (ret = xc_dom_gnttab_init(dom)) != 0 ) {
> +    /* PVH sets up its own grant during boot via hvm mechanisms */
> +    if ( !dom->pvh_enabled && (ret = xc_dom_gnttab_init(dom)) != 0 ) {

I think xc_dom_gnttab_init should silently return if dom->pvh_enabled.

>          LOGE(ERROR, "xc_dom_gnttab_init failed");
>          goto out;
>      }
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index f051d91..441ec66 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -886,6 +886,7 @@ typedef struct {
>      libxl__file_reference pv_kernel;
>      libxl__file_reference pv_ramdisk;
>      const char * pv_cmdline;
> +    bool pvh_enabled;
>  } libxl__domain_build_state;
>  
>  _hidden int libxl__build_pre(libxl__gc *gc, uint32_t domid,
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 85341a0..9919786 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -248,6 +248,7 @@ libxl_domain_create_info = Struct("domain_create_info",[
>      ("platformdata", libxl_key_value_list),
>      ("poolid",       uint32),
>      ("run_hotplug_scripts",libxl_defbool),
> +    ("pvh",          libxl_defbool),
>      ], dir=DIR_IN)
>  
>  MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
> diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
> index a78c91d..87a8110 100644
> --- a/tools/libxl/libxl_x86.c
> +++ b/tools/libxl/libxl_x86.c
> @@ -290,7 +290,9 @@ int libxl__arch_domain_create(libxl__gc *gc, 
> libxl_domain_config *d_config,
>      if (rtc_timeoffset)
>          xc_domain_set_time_offset(ctx->xch, domid, rtc_timeoffset);
>  
> -    if (d_config->b_info.type == LIBXL_DOMAIN_TYPE_HVM) {
> +    if (d_config->b_info.type == LIBXL_DOMAIN_TYPE_HVM ||
> +        libxl_defbool_val(d_config->c_info.pvh)) {
> +
>          unsigned long shadow;
>          shadow = (d_config->b_info.shadow_memkb + 1023) / 1024;
>          xc_shadow_control(ctx->xch, domid, 
> XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION, NULL, 0, &shadow, 0, NULL);
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 884f050..2ee68a9 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -610,6 +610,7 @@ static void parse_config_data(const char *config_source,
>          !strncmp(buf, "hvm", strlen(buf)))
>          c_info->type = LIBXL_DOMAIN_TYPE_HVM;
>  
> +    xlu_cfg_get_defbool(config, "pvh", &c_info->pvh, 0);
>      xlu_cfg_get_defbool(config, "hap", &c_info->hap, 0);
>  
>      if (xlu_cfg_replace_string (config, "name", &c_info->name, 0)) {



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