diff mbox series

[XEN,for-4.13,v7,04/11] libxl: libxl_domain_need_memory: Make it take a domain_config

Message ID 20191023130013.32382-5-ian.jackson@eu.citrix.com (mailing list archive)
State New, archived
Headers show
Series libxl memkb & pt defaulting | expand

Commit Message

Ian Jackson Oct. 23, 2019, 1 p.m. UTC 
This should calculate the extra memory needed for shadow and iommu,
the defaults for which depend on values in c_info.  So we need this to
have the complete domain config available.

And the defaults should actually be updated and stored.  So make it
non-const.

We provide the usual kind of compatibility function for callers
expecting 4.12 and earlier.  This function becomes responsible for the
clone-and-modify of the b_info.

No overall functional change for external libxl callers which use the
API version system to request a particular API version.

Other external libxl callers will need to update their calling code,
and will then find that the new version of this function fills in most
of the defaults in d_config.  Because libxl__domain_config_setdefault
doesn't quite do all of the defaults, that's only partial.  For
present purposes that doesn't matter because none of the missing
settings are used by the memory calculations.  It does mean we need to
document in the API spec that the defaulting is only partial.

This lack of functional change is despite the fact that
numa_place_domain now no longer calls
libxl__domain_build_info_setdefault (via libxl_domain_need_memory).
That is OK because it's idempotent and numa_place_domain's one call
site is libxl__build_pre which is called from libxl__domain_build
which is called from domcreate_bootloader_done, well after the
defaults are set by initiate_domain_create.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
---
v2: Drop now-erroneous GC_FREE as well as the corresponding GC_INIT.
---
 tools/libxl/libxl.h          | 23 +++++++++++++++-
 tools/libxl/libxl_dom.c      |  7 +++--
 tools/libxl/libxl_internal.h |  4 +++
 tools/libxl/libxl_mem.c      | 65 +++++++++++++++++++++++++++++++++++---------
 tools/xl/xl_vmcontrol.c      |  2 +-
 5 files changed, 83 insertions(+), 18 deletions(-)

Comments

Jürgen Groß Oct. 23, 2019, 1:41 p.m. UTC | #1 
On 23.10.19 15:00, Ian Jackson wrote:
> This should calculate the extra memory needed for shadow and iommu,
> the defaults for which depend on values in c_info.  So we need this to
> have the complete domain config available.
> 
> And the defaults should actually be updated and stored.  So make it
> non-const.
> 
> We provide the usual kind of compatibility function for callers
> expecting 4.12 and earlier.  This function becomes responsible for the
> clone-and-modify of the b_info.
> 
> No overall functional change for external libxl callers which use the
> API version system to request a particular API version.
> 
> Other external libxl callers will need to update their calling code,
> and will then find that the new version of this function fills in most
> of the defaults in d_config.  Because libxl__domain_config_setdefault
> doesn't quite do all of the defaults, that's only partial.  For
> present purposes that doesn't matter because none of the missing
> settings are used by the memory calculations.  It does mean we need to
> document in the API spec that the defaulting is only partial.
> 
> This lack of functional change is despite the fact that
> numa_place_domain now no longer calls
> libxl__domain_build_info_setdefault (via libxl_domain_need_memory).
> That is OK because it's idempotent and numa_place_domain's one call
> site is libxl__build_pre which is called from libxl__domain_build
> which is called from domcreate_bootloader_done, well after the
> defaults are set by initiate_domain_create.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen
diff mbox series

Patch

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 518fc9e47f..49b56fa1a3 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1245,6 +1245,20 @@  void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
  */
 #define LIBXL_HAVE_FN_USING_QMP_ASYNC 1
 
+/*
+ * LIBXL_HAVE_DOMAIN_NEED_MEMORY_CONFIG
+ *
+ * If this is set, libxl_domain_need_memory takes a
+ * libxl_domain_config* (non-const) and uint32_t domid_for_logging
+ * (instead of a const libxl_domain_build_info*).
+ *
+ * If this is set, there is no need to call
+ * libxl_get_required_shadow_memory and instead the caller should
+ * simply leave shadow_memkb set to LIBXL_MEMKB_DEFAULT and allow
+ * libxl to fill in a suitable default in the usual way.
+ */
+#define LIBXL_HAVE_DOMAIN_NEED_MEMORY_CONFIG
+
 typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);
@@ -1723,8 +1737,13 @@  int libxl_get_memory_target_0x040700(libxl_ctx *ctx, uint32_t domid,
  */
 /* how much free memory in the system a domain needs to be built */
 int libxl_domain_need_memory(libxl_ctx *ctx,
-                             const libxl_domain_build_info *b_info_in,
+                             libxl_domain_config *config
+                             /* ^ will be partially defaulted */,
+                             uint32_t domid_for_logging /* INVALID_DOMID ok */,
                              uint64_t *need_memkb);
+int libxl_domain_need_memory_0x041200(libxl_ctx *ctx,
+                                      const libxl_domain_build_info *b_info_in,
+                                      uint64_t *need_memkb);
 int libxl_domain_need_memory_0x040700(libxl_ctx *ctx,
                                       const libxl_domain_build_info *b_info_in,
                                       uint32_t *need_memkb)
@@ -1754,6 +1773,8 @@  int libxl_wait_for_memory_target(libxl_ctx *ctx, uint32_t domid, int wait_secs);
 #define libxl_get_memory_target libxl_get_memory_target_0x040700
 #define libxl_domain_need_memory libxl_domain_need_memory_0x040700
 #define libxl_get_free_memory libxl_get_free_memory_0x040700
+#elif defined(LIBXL_API_VERSION) && LIBXL_API_VERSION < 0x041300
+#define libxl_domain_need_memory libxl_domain_need_memory_0x041200
 #endif
 
 int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index c5685b061c..cdb294ab8d 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -140,8 +140,9 @@  static int numa_cmpf(const libxl__numa_candidate *c1,
 
 /* The actual automatic NUMA placement routine */
 static int numa_place_domain(libxl__gc *gc, uint32_t domid,
-                             libxl_domain_build_info *info)
+                             libxl_domain_config *d_config)
 {
+    libxl_domain_build_info *info = &d_config->b_info;
     int found;
     libxl__numa_candidate candidate;
     libxl_bitmap cpumap, cpupool_nodemap, *map;
@@ -195,7 +196,7 @@  static int numa_place_domain(libxl__gc *gc, uint32_t domid,
         }
     }
 
-    rc = libxl_domain_need_memory(CTX, info, &memkb);
+    rc = libxl__domain_need_memory_calculate(gc, info, &memkb);
     if (rc)
         goto out;
     if (libxl_node_bitmap_alloc(CTX, &cpupool_nodemap, 0)) {
@@ -432,7 +433,7 @@  int libxl__build_pre(libxl__gc *gc, uint32_t domid,
             if (rc)
                 return rc;
 
-            rc = numa_place_domain(gc, domid, info);
+            rc = numa_place_domain(gc, domid, d_config);
             if (rc) {
                 libxl_bitmap_dispose(&cpumap_soft);
                 return rc;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 50ac7b64ed..01de5576d9 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1450,6 +1450,10 @@  _hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
 _hidden void libxl__rdm_setdefault(libxl__gc *gc,
                                    libxl_domain_build_info *b_info);
 
+_hidden int libxl__domain_need_memory_calculate(libxl__gc *gc,
+                                      libxl_domain_build_info *b_info,
+                                      uint64_t *need_memkb);
+
 _hidden const char *libxl__device_nic_devname(libxl__gc *gc,
                                               uint32_t domid,
                                               uint32_t devid,
diff --git a/tools/libxl/libxl_mem.c b/tools/libxl/libxl_mem.c
index fd6f33312e..6042299393 100644
--- a/tools/libxl/libxl_mem.c
+++ b/tools/libxl/libxl_mem.c
@@ -446,20 +446,12 @@  int libxl_get_memory_target_0x040700(
     return libxl__memkb_64to32(ctx, rc, my_out_target, out_target);
 }
 
-int libxl_domain_need_memory(libxl_ctx *ctx,
-                             const libxl_domain_build_info *b_info_in,
-                             uint64_t *need_memkb)
+int libxl__domain_need_memory_calculate(libxl__gc *gc,
+                              libxl_domain_build_info *b_info,
+                              uint64_t *need_memkb)
 {
-    GC_INIT(ctx);
-    libxl_domain_build_info b_info[1];
     int rc;
 
-    libxl_domain_build_info_init(b_info);
-    libxl_domain_build_info_copy(ctx, b_info, b_info_in);
-
-    rc = libxl__domain_build_info_setdefault(gc, b_info);
-    if (rc) goto out;
-
     *need_memkb = b_info->target_memkb;
     *need_memkb += b_info->shadow_memkb + b_info->iommu_memkb;
 
@@ -481,10 +473,57 @@  int libxl_domain_need_memory(libxl_ctx *ctx,
         *need_memkb += (2 * 1024) - (*need_memkb % (2 * 1024));
     rc = 0;
 out:
+    return rc;
+}
+
+int libxl_domain_need_memory(libxl_ctx *ctx,
+                             libxl_domain_config *d_config,
+                             uint32_t domid_for_logging,
+                             uint64_t *need_memkb)
+{
+    GC_INIT(ctx);
+    int rc;
+
+    rc = libxl__domain_config_setdefault(gc,
+                                         d_config,
+                                         domid_for_logging);
+    if (rc) goto out;
+
+    rc = libxl__domain_need_memory_calculate(gc,
+                                   &d_config->b_info,
+                                   need_memkb);
+    if (rc) goto out;
+
+    rc = 0;
+ out:
     GC_FREE;
-    libxl_domain_build_info_dispose(b_info);
     return rc;
+}
+
+int libxl_domain_need_memory_0x041200(libxl_ctx *ctx,
+                                      const libxl_domain_build_info *b_info_in,
+                                      uint64_t *need_memkb)
+{
+    GC_INIT(ctx);
+    int rc;
+
+    libxl_domain_build_info b_info[1];
+    libxl_domain_build_info_init(b_info);
+    libxl_domain_build_info_copy(ctx, b_info, b_info_in);
+
+    rc = libxl__domain_build_info_setdefault(gc, b_info);
+    if (rc) goto out;
 
+    rc = libxl__domain_need_memory_calculate(gc,
+                                   b_info,
+                                   need_memkb);
+    if (rc) goto out;
+
+    rc = 0;
+ out:
+    libxl_domain_build_info_dispose(b_info);
+    GC_FREE;
+    return rc;
 }
 
 int libxl_domain_need_memory_0x040700(libxl_ctx *ctx,
@@ -494,7 +533,7 @@  int libxl_domain_need_memory_0x040700(libxl_ctx *ctx,
     uint64_t my_need_memkb;
     int rc;
 
-    rc = libxl_domain_need_memory(ctx, b_info_in, &my_need_memkb);
+    rc = libxl_domain_need_memory_0x041200(ctx, b_info_in, &my_need_memkb);
     return libxl__memkb_64to32(ctx, rc, my_need_memkb, need_memkb);
 }
 
diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
index d33c6b38c9..e520b1da79 100644
--- a/tools/xl/xl_vmcontrol.c
+++ b/tools/xl/xl_vmcontrol.c
@@ -322,7 +322,7 @@  static bool freemem(uint32_t domid, libxl_domain_config *d_config)
     if (!autoballoon)
         return true;
 
-    rc = libxl_domain_need_memory(ctx, &d_config->b_info, &need_memkb);
+    rc = libxl_domain_need_memory(ctx, d_config, domid, &need_memkb);
     if (rc < 0)
         return false;