* [PATCH] tools/libxl: run libxl__arch_domain_create() much earlier.
@ 2013-03-14 12:39 Tim Deegan
2013-03-28 9:49 ` Tim Deegan
2013-03-28 15:53 ` Ian Jackson
0 siblings, 2 replies; 3+ messages in thread
From: Tim Deegan @ 2013-03-14 12:39 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Ian Campbell, Stefano Stabellini
Among other things, arch_domain_create() sets the shadow(/hap/p2m)
memory allocation, which must happen after vcpus are assigned (or the
shadow op will fail) but before memory is allocated (or we might run
out of p2m memory).
libxl__build_pre(), which already sets similar things like maxmem,
semes like a reasonable spot for it. That needed a bit of plumbing to
get the right datastructure from the caller.
As a side-effect, the return code from libxl__arch_domain_create() is
no longer ignored.
Reported-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Tim Deegan <tim@xen.org>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_create.c | 10 +++++-----
tools/libxl/libxl_dm.c | 2 +-
tools/libxl/libxl_dom.c | 10 +++++++---
tools/libxl/libxl_internal.h | 5 +++--
4 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 2ea628a..30a4507 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -315,15 +315,16 @@ static int init_console_info(libxl__device_console *console, int dev_num)
}
int libxl__domain_build(libxl__gc *gc,
- libxl_domain_build_info *info,
+ libxl_domain_config *d_config,
uint32_t domid,
libxl__domain_build_state *state)
{
+ libxl_domain_build_info *const info = &d_config->b_info;
char **vments = NULL, **localents = NULL;
struct timeval start_time;
int i, ret;
- ret = libxl__build_pre(gc, domid, info, state);
+ ret = libxl__build_pre(gc, domid, d_config, state);
if (ret)
goto out;
@@ -750,14 +751,14 @@ static void domcreate_bootloader_done(libxl__egc *egc,
dcs->dmss.callback = domcreate_devmodel_started;
if ( restore_fd < 0 ) {
- rc = libxl__domain_build(gc, &d_config->b_info, domid, state);
+ rc = libxl__domain_build(gc, d_config, domid, state);
domcreate_rebuild_done(egc, dcs, rc);
return;
}
/* Restore */
- rc = libxl__build_pre(gc, domid, info, state);
+ rc = libxl__build_pre(gc, domid, d_config, state);
if (rc)
goto out;
@@ -1169,7 +1170,6 @@ static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *multidev,
}
}
- libxl__arch_domain_create(gc, d_config, domid);
domcreate_console_available(egc, dcs);
domcreate_complete(egc, dcs, 0);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index a8a36d7..3969975 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -828,7 +828,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
if (ret)
goto out;
uint32_t dm_domid = sdss->pvqemu.guest_domid;
- ret = libxl__domain_build(gc, &dm_config->b_info, dm_domid, stubdom_state);
+ ret = libxl__domain_build(gc, dm_config, dm_domid, stubdom_state);
if (ret)
goto out;
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index de555ee..2dd429f 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -18,6 +18,7 @@
#include <glob.h>
#include "libxl_internal.h"
+#include "libxl_arch.h"
#include <xc_dom.h>
#include <xen/hvm/hvm_info_table.h>
@@ -199,10 +200,12 @@ static int numa_place_domain(libxl__gc *gc, uint32_t domid,
}
int libxl__build_pre(libxl__gc *gc, uint32_t domid,
- libxl_domain_build_info *info, libxl__domain_build_state *state)
+ libxl_domain_config *d_config, libxl__domain_build_state *state)
{
+ libxl_domain_build_info *const info = &d_config->b_info;
libxl_ctx *ctx = libxl__gc_owner(gc);
char *xs_domid, *con_domid;
+ int rc;
xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus);
@@ -216,7 +219,6 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
* whatever that turns out to be.
*/
if (libxl_defbool_val(info->numa_placement)) {
- int rc;
if (!libxl_bitmap_is_full(&info->cpumap)) {
LOG(ERROR, "Can run NUMA placement only if no vcpu "
@@ -243,7 +245,9 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
state->console_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->console_domid);
state->vm_generationid_addr = 0;
- return 0;
+ rc = libxl__arch_domain_create(gc, d_config, domid);
+
+ return rc;
}
int libxl__build_post(libxl__gc *gc, uint32_t domid,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 8be086d..3ba3a21 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -884,7 +884,8 @@ typedef struct {
} libxl__domain_build_state;
_hidden int libxl__build_pre(libxl__gc *gc, uint32_t domid,
- libxl_domain_build_info *info, libxl__domain_build_state *state);
+ libxl_domain_config * const d_config,
+ libxl__domain_build_state *state);
_hidden int libxl__build_post(libxl__gc *gc, uint32_t domid,
libxl_domain_build_info *info, libxl__domain_build_state *state,
char **vms_ents, char **local_ents);
@@ -1278,7 +1279,7 @@ _hidden int libxl__domain_make(libxl__gc *gc,
uint32_t *domid);
_hidden int libxl__domain_build(libxl__gc *gc,
- libxl_domain_build_info *info,
+ libxl_domain_config *d_config,
uint32_t domid,
libxl__domain_build_state *state);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] tools/libxl: run libxl__arch_domain_create() much earlier.
2013-03-14 12:39 [PATCH] tools/libxl: run libxl__arch_domain_create() much earlier Tim Deegan
@ 2013-03-28 9:49 ` Tim Deegan
2013-03-28 15:53 ` Ian Jackson
1 sibling, 0 replies; 3+ messages in thread
From: Tim Deegan @ 2013-03-28 9:49 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Ian Campbell, Stefano Stabellini
Ping?
At 12:39 +0000 on 14 Mar (1363264779), Tim Deegan wrote:
> Among other things, arch_domain_create() sets the shadow(/hap/p2m)
> memory allocation, which must happen after vcpus are assigned (or the
> shadow op will fail) but before memory is allocated (or we might run
> out of p2m memory).
>
> libxl__build_pre(), which already sets similar things like maxmem,
> semes like a reasonable spot for it. That needed a bit of plumbing to
> get the right datastructure from the caller.
>
> As a side-effect, the return code from libxl__arch_domain_create() is
> no longer ignored.
>
> Reported-by: Jan Beulich <JBeulich@suse.com>
> Signed-off-by: Tim Deegan <tim@xen.org>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> ---
> tools/libxl/libxl_create.c | 10 +++++-----
> tools/libxl/libxl_dm.c | 2 +-
> tools/libxl/libxl_dom.c | 10 +++++++---
> tools/libxl/libxl_internal.h | 5 +++--
> 4 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 2ea628a..30a4507 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -315,15 +315,16 @@ static int init_console_info(libxl__device_console *console, int dev_num)
> }
>
> int libxl__domain_build(libxl__gc *gc,
> - libxl_domain_build_info *info,
> + libxl_domain_config *d_config,
> uint32_t domid,
> libxl__domain_build_state *state)
> {
> + libxl_domain_build_info *const info = &d_config->b_info;
> char **vments = NULL, **localents = NULL;
> struct timeval start_time;
> int i, ret;
>
> - ret = libxl__build_pre(gc, domid, info, state);
> + ret = libxl__build_pre(gc, domid, d_config, state);
> if (ret)
> goto out;
>
> @@ -750,14 +751,14 @@ static void domcreate_bootloader_done(libxl__egc *egc,
> dcs->dmss.callback = domcreate_devmodel_started;
>
> if ( restore_fd < 0 ) {
> - rc = libxl__domain_build(gc, &d_config->b_info, domid, state);
> + rc = libxl__domain_build(gc, d_config, domid, state);
> domcreate_rebuild_done(egc, dcs, rc);
> return;
> }
>
> /* Restore */
>
> - rc = libxl__build_pre(gc, domid, info, state);
> + rc = libxl__build_pre(gc, domid, d_config, state);
> if (rc)
> goto out;
>
> @@ -1169,7 +1170,6 @@ static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *multidev,
> }
> }
>
> - libxl__arch_domain_create(gc, d_config, domid);
> domcreate_console_available(egc, dcs);
>
> domcreate_complete(egc, dcs, 0);
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index a8a36d7..3969975 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -828,7 +828,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
> if (ret)
> goto out;
> uint32_t dm_domid = sdss->pvqemu.guest_domid;
> - ret = libxl__domain_build(gc, &dm_config->b_info, dm_domid, stubdom_state);
> + ret = libxl__domain_build(gc, dm_config, dm_domid, stubdom_state);
> if (ret)
> goto out;
>
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index de555ee..2dd429f 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -18,6 +18,7 @@
> #include <glob.h>
>
> #include "libxl_internal.h"
> +#include "libxl_arch.h"
>
> #include <xc_dom.h>
> #include <xen/hvm/hvm_info_table.h>
> @@ -199,10 +200,12 @@ static int numa_place_domain(libxl__gc *gc, uint32_t domid,
> }
>
> int libxl__build_pre(libxl__gc *gc, uint32_t domid,
> - libxl_domain_build_info *info, libxl__domain_build_state *state)
> + libxl_domain_config *d_config, libxl__domain_build_state *state)
> {
> + libxl_domain_build_info *const info = &d_config->b_info;
> libxl_ctx *ctx = libxl__gc_owner(gc);
> char *xs_domid, *con_domid;
> + int rc;
>
> xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus);
>
> @@ -216,7 +219,6 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
> * whatever that turns out to be.
> */
> if (libxl_defbool_val(info->numa_placement)) {
> - int rc;
>
> if (!libxl_bitmap_is_full(&info->cpumap)) {
> LOG(ERROR, "Can run NUMA placement only if no vcpu "
> @@ -243,7 +245,9 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
> state->console_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->console_domid);
> state->vm_generationid_addr = 0;
>
> - return 0;
> + rc = libxl__arch_domain_create(gc, d_config, domid);
> +
> + return rc;
> }
>
> int libxl__build_post(libxl__gc *gc, uint32_t domid,
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 8be086d..3ba3a21 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -884,7 +884,8 @@ typedef struct {
> } libxl__domain_build_state;
>
> _hidden int libxl__build_pre(libxl__gc *gc, uint32_t domid,
> - libxl_domain_build_info *info, libxl__domain_build_state *state);
> + libxl_domain_config * const d_config,
> + libxl__domain_build_state *state);
> _hidden int libxl__build_post(libxl__gc *gc, uint32_t domid,
> libxl_domain_build_info *info, libxl__domain_build_state *state,
> char **vms_ents, char **local_ents);
> @@ -1278,7 +1279,7 @@ _hidden int libxl__domain_make(libxl__gc *gc,
> uint32_t *domid);
>
> _hidden int libxl__domain_build(libxl__gc *gc,
> - libxl_domain_build_info *info,
> + libxl_domain_config *d_config,
> uint32_t domid,
> libxl__domain_build_state *state);
>
> --
> 1.7.10.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] tools/libxl: run libxl__arch_domain_create() much earlier.
2013-03-14 12:39 [PATCH] tools/libxl: run libxl__arch_domain_create() much earlier Tim Deegan
2013-03-28 9:49 ` Tim Deegan
@ 2013-03-28 15:53 ` Ian Jackson
1 sibling, 0 replies; 3+ messages in thread
From: Ian Jackson @ 2013-03-28 15:53 UTC (permalink / raw)
To: Tim Deegan; +Cc: Stefano Stabellini, Ian Campbell, xen-devel@lists.xen.org
Tim Deegan writes ("[PATCH] tools/libxl: run libxl__arch_domain_create() much earlier."):
> Among other things, arch_domain_create() sets the shadow(/hap/p2m)
> memory allocation, which must happen after vcpus are assigned (or the
> shadow op will fail) but before memory is allocated (or we might run
> out of p2m memory).
>
> libxl__build_pre(), which already sets similar things like maxmem,
> semes like a reasonable spot for it. That needed a bit of plumbing to
> get the right datastructure from the caller.
>
> As a side-effect, the return code from libxl__arch_domain_create() is
> no longer ignored.
Applied, thanks.
Thanks for referencing
http://lists.xen.org/archives/html/xen-devel/2013-03/msg00191.html
on irc, Tim. I added a reference to it to the commit message.
Ian.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-03-28 15:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-14 12:39 [PATCH] tools/libxl: run libxl__arch_domain_create() much earlier Tim Deegan
2013-03-28 9:49 ` Tim Deegan
2013-03-28 15:53 ` Ian Jackson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).