Re: [Xen-devel] [PATCH v3 3/5] libxl: initialise the build info before calling prepare_config

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Wed, 2016-01-20 at 12:57 +0100, Roger Pau Monne wrote:
> libxl__arch_domain_prepare_config has access to the
> libxl_domain_build_info
> struct, so make sure it's properly initialised.
> 
> Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
> ---
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
> NB: libxl__arch_domain_prepare_config is called from libxl__domain_make.

I think this is worth moving into the actual commit message. Plus it would
be useful to clarify that while prepare_config has access to b_info it
doesn't touch it right now (but presumably you are about to make it do so).

If it does touch it then that is currently a bug which should be mentioned
in the commit message and tagged for backport etc.

I suspect the reason for the ordering today is that domain_make is intended
to consume create_info, not build_info. However that distinction seems to
me to be an artefact of a much older API structure which doesn't seem to
make much sense now (but we are stuck with it :-()

> ---
> Âtools/libxl/libxl_create.c | 12 ++++++------
> Â1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 61a4001..ba4c9e8 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -892,6 +892,12 @@ static void initiate_domain_create(libxl__egc *egc,
> ÂÂÂÂÂÂÂÂÂgoto error_out;
> ÂÂÂÂÂ}
> Â
> +ÂÂÂÂret = libxl__domain_build_info_setdefault(gc, &d_config->b_info);
> +ÂÂÂÂif (ret) {
> +ÂÂÂÂÂÂÂÂLOG(ERROR, "Unable to set domain build info defaults");
> +ÂÂÂÂÂÂÂÂgoto error_out;
> +ÂÂÂÂ}
> +
> ÂÂÂÂÂret = libxl__domain_make(gc, d_config, &domid, &state->config);
> ÂÂÂÂÂif (ret) {
> ÂÂÂÂÂÂÂÂÂLOG(ERROR, "cannot make domain: %d", ret);
> @@ -903,12 +909,6 @@ static void initiate_domain_create(libxl__egc *egc,
> ÂÂÂÂÂdcs->guest_domid = domid;
> ÂÂÂÂÂdcs->dmss.dm.guest_domid = 0; /* means we haven't spawned */
> Â
> -ÂÂÂÂret = libxl__domain_build_info_setdefault(gc, &d_config->b_info);
> -ÂÂÂÂif (ret) {
> -ÂÂÂÂÂÂÂÂLOG(ERROR, "Unable to set domain build info defaults");
> -ÂÂÂÂÂÂÂÂgoto error_out;
> -ÂÂÂÂ}
> -
> ÂÂÂÂÂif (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
> ÂÂÂÂÂÂÂÂÂ(libxl_defbool_val(d_config->b_info.u.hvm.nested_hvm) &&
> ÂÂÂÂÂÂÂÂÂÂlibxl_defbool_val(d_config->b_info.u.hvm.altp2m))) {

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