thr3ads.net - Xen devel - [PATCH] tools/libxl: run libxl__arch_domain

If this information is useful, please help other people find it:
Share via:

Tim Deegan

2013-Mar-14 12:39 UTC

[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.

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

Tim Deegan

2013-Mar-28 09:49 UTC

head link

Re: [PATCH] tools/libxl: run libxl__arch_domain_create() much earlier.

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

Ian Jackson

2013-Mar-28 15:53 UTC

head link

Re: [PATCH] tools/libxl: run libxl__arch_domain_create() much earlier.

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.

Xen devel - Mar 2013 - [PATCH] tools/libxl: run libxl__arch_domain_create() much earlier.

[PATCH] tools/libxl: run libxl__arch_domain_create() much earlier.

Re: [PATCH] tools/libxl: run libxl__arch_domain_create() much earlier.

Re: [PATCH] tools/libxl: run libxl__arch_domain_create() much earlier.