| Message ID | 20191004151707.24844-4-ian.jackson@eu.citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Drop/deprecate libxl_get_required_*_memory | expand |
On Fri, Oct 04, 2019 at 04:17:04PM +0100, Ian Jackson wrote: > Break out this into a new function. We are going to want to call it > from a new call site. > > Unfortunately not all of the defaults can be moved into the new > function without changing the order in which things are done. That > does not seem wise at this stage of the release. The effect is that > additional calls to libxl__domain_config_setdefault (which are going > to be introduced) do not quite set everything. But they will do what > is needed. After Xen 4.13 is done, we should move those settings into > the right order. > > No functional change. > > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> > --- > tools/libxl/libxl_create.c | 40 ++++++++++++++++++++++++++++------------ > tools/libxl/libxl_internal.h | 3 +++ > 2 files changed, 31 insertions(+), 12 deletions(-) > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > index 099761a2d7..457beb6462 100644 > --- a/tools/libxl/libxl_create.c > +++ b/tools/libxl/libxl_create.c > @@ -862,22 +862,14 @@ static void domcreate_destruction_cb(libxl__egc *egc, > libxl__domain_destroy_state *dds, > int rc); > > -static void initiate_domain_create(libxl__egc *egc, > - libxl__domain_create_state *dcs) > +int libxl__domain_config_setdefault(libxl__gc *gc, > + libxl_domain_config *d_config, > + uint32_t domid) > { > - STATE_AO_GC(dcs->ao); > libxl_ctx *ctx = libxl__gc_owner(gc); > - uint32_t domid; > - int i, ret; > + int ret; > bool pod_enabled = false; > > - /* convenience aliases */ > - libxl_domain_config *const d_config = dcs->guest_config; > - const int restore_fd = dcs->restore_fd; > - > - domid = dcs->domid_soft_reset; > - libxl__domain_build_state_init(&dcs->build_state); > - > if (d_config->c_info.ssid_label) { > char *s = d_config->c_info.ssid_label; > ret = libxl_flask_context_to_sid(ctx, s, strlen(s), > @@ -1008,6 +1000,27 @@ static void initiate_domain_create(libxl__egc *egc, > goto error_out; > } > > + ret = 0; > + error_out: > + return ret; > +} > + > +static void initiate_domain_create(libxl__egc *egc, > + libxl__domain_create_state *dcs) > +{ > + STATE_AO_GC(dcs->ao); > + uint32_t domid; > + int i, ret; > + > + /* convenience aliases */ > + libxl_domain_config *const d_config = dcs->guest_config; > + const int restore_fd = dcs->restore_fd; > + > + domid = dcs->domid_soft_reset; > + libxl__domain_build_state_init(&dcs->build_state); > + > + libxl__domain_config_setdefault(gc,d_config,domid); Shouldn't you check for error ? Thanks,
Anthony PERARD writes ("Re: [XEN PATCH for-4.13 3/6] libxl: libxl__domain_config_setdefault: New function"):
> On Fri, Oct 04, 2019 at 04:17:04PM +0100, Ian Jackson wrote:
> > + libxl__domain_config_setdefault(gc,d_config,domid);
>
> Shouldn't you check for error ?
Blimey, yes! Thanks!
Ian.
On Fri, Oct 04, 2019 at 05:04:27PM +0100, Ian Jackson wrote: > Anthony PERARD writes ("Re: [XEN PATCH for-4.13 3/6] libxl: libxl__domain_config_setdefault: New function"): > > On Fri, Oct 04, 2019 at 04:17:04PM +0100, Ian Jackson wrote: > > > + libxl__domain_config_setdefault(gc,d_config,domid); > > > > Shouldn't you check for error ? > > Blimey, yes! Thanks! With that fixed: Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Anthony PERARD writes ("Re: [XEN PATCH for-4.13 3/6] libxl: libxl__domain_config_setdefault: New function"):
> On Fri, Oct 04, 2019 at 05:04:27PM +0100, Ian Jackson wrote:
> > Anthony PERARD writes ("Re: [XEN PATCH for-4.13 3/6] libxl: libxl__domain_config_setdefault: New function"):
> > > Shouldn't you check for error ?
> >
> > Blimey, yes! Thanks!
>
> With that fixed:
> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Thanks.
I think the things you spotted in this review means I should go
through this series again, and try to test it somehow. I will do that
and then send a v2.
Ian.
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 099761a2d7..457beb6462 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -862,22 +862,14 @@ static void domcreate_destruction_cb(libxl__egc *egc, libxl__domain_destroy_state *dds, int rc); -static void initiate_domain_create(libxl__egc *egc, - libxl__domain_create_state *dcs) +int libxl__domain_config_setdefault(libxl__gc *gc, + libxl_domain_config *d_config, + uint32_t domid) { - STATE_AO_GC(dcs->ao); libxl_ctx *ctx = libxl__gc_owner(gc); - uint32_t domid; - int i, ret; + int ret; bool pod_enabled = false; - /* convenience aliases */ - libxl_domain_config *const d_config = dcs->guest_config; - const int restore_fd = dcs->restore_fd; - - domid = dcs->domid_soft_reset; - libxl__domain_build_state_init(&dcs->build_state); - if (d_config->c_info.ssid_label) { char *s = d_config->c_info.ssid_label; ret = libxl_flask_context_to_sid(ctx, s, strlen(s), @@ -1008,6 +1000,27 @@ static void initiate_domain_create(libxl__egc *egc, goto error_out; } + ret = 0; + error_out: + return ret; +} + +static void initiate_domain_create(libxl__egc *egc, + libxl__domain_create_state *dcs) +{ + STATE_AO_GC(dcs->ao); + uint32_t domid; + int i, ret; + + /* convenience aliases */ + libxl_domain_config *const d_config = dcs->guest_config; + const int restore_fd = dcs->restore_fd; + + domid = dcs->domid_soft_reset; + libxl__domain_build_state_init(&dcs->build_state); + + libxl__domain_config_setdefault(gc,d_config,domid); + ret = libxl__domain_make(gc, d_config, &dcs->build_state, &domid); if (ret) { LOGD(ERROR, domid, "cannot make domain: %d", ret); @@ -1019,6 +1032,9 @@ static void initiate_domain_create(libxl__egc *egc, dcs->guest_domid = domid; dcs->sdss.dm.guest_domid = 0; /* means we haven't spawned */ + /* post-4.13 todo: move these next bits of defaulting to + * libxl__domain_config_setdefault */ + /* * Set the dm version quite early so that libxl doesn't have to pass the * build info around just to know if the domain has a device model or not. diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index d2d5af746b..50ac7b64ed 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -1440,6 +1440,9 @@ _hidden int libxl__resolve_domid(libxl__gc *gc, const char *name, * All libxl API functions are expected to have arranged for this * to be called before using any values within these structures. */ +_hidden int libxl__domain_config_setdefault(libxl__gc *gc, + libxl_domain_config *d_config, + uint32_t domid /* logging only */); _hidden int libxl__domain_create_info_setdefault(libxl__gc *gc, libxl_domain_create_info *c_info); _hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
Break out this into a new function. We are going to want to call it from a new call site. Unfortunately not all of the defaults can be moved into the new function without changing the order in which things are done. That does not seem wise at this stage of the release. The effect is that additional calls to libxl__domain_config_setdefault (which are going to be introduced) do not quite set everything. But they will do what is needed. After Xen 4.13 is done, we should move those settings into the right order. No functional change. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> --- tools/libxl/libxl_create.c | 40 ++++++++++++++++++++++++++++------------ tools/libxl/libxl_internal.h | 3 +++ 2 files changed, 31 insertions(+), 12 deletions(-)