xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Ian Jackson <ian.jackson@eu.citrix.com>
To: <xen-devel@lists.xenproject.org>
Cc: Wei Liu <wl@xen.org>, Andrew Cooper <Andrew.Cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Paul Durrant <pdurrant@gmail.com>,
	Jan Beulich <jbeulich@suse.com>,
	Anthony PERARD <anthony.perard@citrix.com>
Subject: [Xen-devel] [XEN PATCH for-4.13 v2 4/9] libxl: libxl_domain_need_memory: Make it take a domain_config
Date: Thu, 10 Oct 2019 16:11:06 +0100	[thread overview]
Message-ID: <20191010151111.22125-5-ian.jackson@eu.citrix.com> (raw)
In-Reply-To: <20191010151111.22125-1-ian.jackson@eu.citrix.com>

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(-)

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;
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  parent reply	other threads:[~2019-10-10 15:11 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-10 15:11 [Xen-devel] [XEN PATCH for-4.13 v2 0/9] libxl memkb & pt defaulting Ian Jackson
2019-10-10 15:11 ` [Xen-devel] [XEN PATCH for-4.13 v2 1/9] libxl: Offer API versions 0x040700 and 0x040800 Ian Jackson
2019-11-25 18:44   ` Jim Fehlig
2019-10-10 15:11 ` [Xen-devel] [XEN PATCH for-4.13 v2 2/9] xl: Pass libxl_domain_config to freemem(), instead of b_info Ian Jackson
2019-10-10 15:11 ` [Xen-devel] [XEN PATCH for-4.13 v2 3/9] libxl: libxl__domain_config_setdefault: New function Ian Jackson
2019-10-10 15:11 ` Ian Jackson [this message]
2019-10-10 15:11 ` [Xen-devel] [XEN PATCH for-4.13 v2 5/9] libxl: Move shadow_memkb and iommu_memkb defaulting into libxl Ian Jackson
2019-10-10 15:11 ` [Xen-devel] [XEN PATCH for-4.13 v2 6/9] libxl: Remove/deprecate libxl_get_required_*_memory from the API Ian Jackson
2019-10-10 15:11 ` [Xen-devel] [XEN PATCH for-4.13 v2 7/9] libxl: create: setdefault: Make libxl_physinfo info[1] Ian Jackson
2019-10-11  9:26   ` Wei Liu
2019-10-10 15:11 ` [Xen-devel] [XEN PATCH for-4.13 v2 8/9] libxl: create: setdefault: Move physinfo into config_setdefault Ian Jackson
2019-10-11  9:26   ` Wei Liu
2019-10-10 15:11 ` [Xen-devel] [XEN PATCH for-4.13 v2 9/9] libxl/xl: Overhaul passthrough setting logic Ian Jackson
2019-10-11  9:26   ` Wei Liu
2019-10-11  9:47   ` Andrew Cooper
2019-10-11 10:10     ` Ian Jackson
2019-10-11 11:00     ` Julien Grall
2019-10-11 13:33       ` Ian Jackson
2019-10-11 12:23   ` George Dunlap
2019-10-11 13:31     ` Ian Jackson
2019-10-11 13:55       ` Jürgen Groß
2019-10-11 16:34         ` Ian Jackson
2019-10-14  7:59           ` Paul Durrant
2019-10-14 16:09             ` Ian Jackson
2019-10-14 16:44               ` Wei Liu
2019-10-14 16:48                 ` Ian Jackson
2019-10-14 16:51                   ` [Xen-devel] [XEN PATCH v4 for-4.13 10/10] libxl/xl: Overhaul passthrough setting logic " Ian Jackson
2019-10-14 17:06                     ` Anthony PERARD
2019-10-14 16:59               ` [Xen-devel] [XEN PATCH for-4.13 v2 9/9] libxl/xl: Overhaul " Anthony PERARD
2019-10-15 11:13                 ` Wei Liu
2019-10-10 15:18 ` [Xen-devel] [XEN PATCH for-4.13 v2 0/9] libxl memkb & pt defaulting Ian Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191010151111.22125-5-ian.jackson@eu.citrix.com \
    --to=ian.jackson@eu.citrix.com \
    --cc=Andrew.Cooper3@citrix.com \
    --cc=anthony.perard@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=pdurrant@gmail.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).