| Message ID | 1453291044-83976-4-git-send-email-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@citrix.com> > --- > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > --- > 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))) {
El 20/01/16 a les 13.46, Ian Campbell ha escrit: > 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@citrix.com> >> --- >> Cc: Ian Jackson <ian.jackson@eu.citrix.com> >> Cc: Ian Campbell <ian.campbell@citrix.com> >> Cc: Wei Liu <wei.liu2@citrix.com> >> --- >> 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). Right, see 5/5. > If it does touch it then that is currently a bug which should be mentioned > in the commit message and tagged for backport etc. No, it doesn't touch it ATM, so not a bug. > 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 :-() That's right, this create_info vs build_info split seems quite arbitrary, but I don't see myself shaving that yak right now (also changing this is going to be a pain regarding API compatibility). Roger.
On Wed, 2016-01-20 at 16:32 +0100, Roger Pau Monné wrote: > > 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 :-() > > That's right, this create_info vs build_info split seems quite > arbitrary, but I don't see myself shaving that yak right now (also > changing this is going to be a pain regarding API compatibility). Yeah, we can't change that in any sane way I don't think. Ian.
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))) {
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@citrix.com> --- Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> --- NB: libxl__arch_domain_prepare_config is called from libxl__domain_make. --- tools/libxl/libxl_create.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)