[PATCH v16] claim and its friends for allocating multiple self-ballooning guests.

[PATCH v16] claim and its friends for allocating multiple self-ballooning guests.

[Konrad Rzeszutek Wilk]

Changes since v15:
 - Added 'Acked-by: Ian' on many patches.
 - Fixed 'xl claims' call to be smarter about reporting information (reworked
   a patch).
 - Fixed 'xl info', free_memory: to also take into account outstanding claims
   (new patch)
Changes since v14:
 - Added 'xl claims' call. Reintroduced the 'outstanding_pages' patches for libxc and libxl.
 - Broke up one of the patches line-lengths.
Changes since v13:
 - Addressed Ian Jacksons' comments - added extra docs, redid the parsing
   of claim_mode.
 - s/global_claim_mode/claim_mode/
 - Dropped xend patch
 - Dropped 'outstanding_pages' patches for libxc and libxl.
Changes since v12:
 - Addressed Ian Campbells' comments.

Note that some of the patches could probably be squashed. In the interest
of sanity and patience of Ian I choose to not do that - so that he can
be assured that the ones he Acked have not changed since the last posting.

That means the only new patches since v14 are:
 [PATCH 6/7] xl: 'xl claims' print outstanding per domain claims
 [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims

The patch (mmu: Introduce XENMEM_claim_pages (subop of memory ops) is already
in the hypervisor and described in details the problem/solution/alternative
solutions. This builds upon that new hypercall to expand the toolstack to
utilize it.

The patches follow the normal code-flow - the patch to implement the two 
hypercalls: XENMEM_claim_pages and XENMEM_get_outstanding_pages.

Then the patches to utilize them in the libxc. The hypercall's are only utilized
if the toolstack (libxl) sets the claim_mode to 1 (true).

Then the toolstack (libxl + xl) patches. They revolve around three different 
changes:
 1). Add 'claim_mode=0|1' global configuration value that determines
     whether the claim hypercall should be used as part of guest creation.
 2). As part of  'xl info' output how many pages are claimed by different guests.
     This is more of a diagnostic patch.
 3). New 'xl claims' provides per domain outstanding claim value along with
     the guest currently populated count.

These patches are also visible at:

  git://xenbits.xen.org/people/konradwilk/xen.git claim.v16


 docs/man/xl.conf.pod.5         | 43 ++++++++++++++++++++++++++++
 docs/man/xl.pod.1              | 39 +++++++++++++++++++++++++-
 tools/examples/xl.conf         |  6 ++++
 tools/libxc/xc_dom.h           |  1 +
 tools/libxc/xc_dom_x86.c       | 12 ++++++++
 tools/libxc/xc_domain.c        | 31 +++++++++++++++++++++
 tools/libxc/xc_hvm_build_x86.c | 23 ++++++++++++---
 tools/libxc/xenctrl.h          |  7 +++++
 tools/libxc/xenguest.h         |  2 ++
 tools/libxl/libxl.c            | 19 +++++++++++--
 tools/libxl/libxl.h            |  2 +-
 tools/libxl/libxl_create.c     |  2 ++
 tools/libxl/libxl_dom.c        |  3 +-
 tools/libxl/libxl_types.idl    |  3 +-
 tools/libxl/xl.c               |  5 ++++
 tools/libxl/xl.h               |  2 ++
 tools/libxl/xl_cmdimpl.c       | 63 ++++++++++++++++++++++++++++++++++++------
 tools/libxl/xl_cmdtable.c      |  6 ++++
 18 files changed, 251 insertions(+), 18 deletions(-)

Dan Magenheimer (2):
      xc: use XENMEM_claim_pages hypercall during guest creation.
      xc: export outstanding_pages value in xc_dominfo structure.

Konrad Rzeszutek Wilk (5):
      xl: Implement XENMEM_claim_pages support via 'claim_mode' global config
      xl: 'xl info' print outstanding claims if enabled (claim_mode=1 in xl.conf)
      xl: export 'outstanding_pages' value from xcinfo
      xl: 'xl claims' print outstanding per domain claims
      xl: Fix 'free_memory' to include outstanding_claims value.

[PATCH 1/7] xc: use XENMEM_claim_pages hypercall during guest creation.

[Konrad Rzeszutek Wilk]

From: Dan Magenheimer <dan.magenheimer@oracle.com>

We add an extra parameter to the structures passed to the
PV routine (arch_setup_meminit) and HVM routine (setup_guest)
that determines whether the claim hypercall is to be done.

The contents of the 'claim_enabled' is defined as an 'int'
in case the hypercall expands in the future with extra
flags (for example for per-NUMA allocation). For right now
the proper values are: 0 to disable it or 1 to enable
it.

If the hypervisor does not support this function, the
xc_domain_claim_pages and xc_domain_get_outstanding_pages
will silently return 0 (and set errno to zero).

Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
[v2: Updated per Ian's recommendations]
[v3: Added support for out-of-sync hypervisor]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxc/xc_dom.h           |  1 +
 tools/libxc/xc_dom_x86.c       | 12 ++++++++++++
 tools/libxc/xc_domain.c        | 30 ++++++++++++++++++++++++++++++
 tools/libxc/xc_hvm_build_x86.c | 23 +++++++++++++++++++----
 tools/libxc/xenctrl.h          |  6 ++++++
 tools/libxc/xenguest.h         |  2 ++
 6 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
index 779b9d4..ac36600 100644
--- a/tools/libxc/xc_dom.h
+++ b/tools/libxc/xc_dom.h
@@ -135,6 +135,7 @@ struct xc_dom_image {
     domid_t guest_domid;
     int8_t vhpt_size_log2; /* for IA64 */
     int8_t superpages;
+    int claim_enabled; /* 0 by default, 1 enables it */
     int shadow_enabled;
 
     int xen_version;
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index eb9ac07..d89526d 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -706,6 +706,13 @@ int arch_setup_meminit(struct xc_dom_image *dom)
     }
     else
     {
+        /* try to claim pages for early warning of insufficient memory avail */
+        if ( dom->claim_enabled ) {
+            rc = xc_domain_claim_pages(dom->xch, dom->guest_domid,
+                                       dom->total_pages);
+            if ( rc )
+                return rc;
+        }
         /* setup initial p2m */
         for ( pfn = 0; pfn < dom->total_pages; pfn++ )
             dom->p2m_host[pfn] = pfn;
@@ -722,6 +729,11 @@ int arch_setup_meminit(struct xc_dom_image *dom)
                 dom->xch, dom->guest_domid, allocsz,
                 0, 0, &dom->p2m_host[i]);
         }
+
+        /* Ensure no unclaimed pages are left unused.
+         * OK to call if hadn't done the earlier claim call. */
+        (void)xc_domain_claim_pages(dom->xch, dom->guest_domid,
+                                    0 /* cancels the claim */);
     }
 
     return rc;
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 480ce91..299c907 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -775,6 +775,36 @@ int xc_domain_add_to_physmap(xc_interface *xch,
     return do_memory_op(xch, XENMEM_add_to_physmap, &xatp, sizeof(xatp));
 }
 
+int xc_domain_claim_pages(xc_interface *xch,
+                               uint32_t domid,
+                               unsigned long nr_pages)
+{
+    int err;
+    struct xen_memory_reservation reservation = {
+        .nr_extents   = nr_pages,
+        .extent_order = 0,
+        .mem_flags    = 0, /* no flags */
+        .domid        = domid
+    };
+
+    set_xen_guest_handle(reservation.extent_start, HYPERCALL_BUFFER_NULL);
+
+    err = do_memory_op(xch, XENMEM_claim_pages, &reservation, sizeof(reservation));
+    /* Ignore it if the hypervisor does not support the call. */
+    if (err == -1 && errno == ENOSYS)
+        err = errno = 0;
+    return err;
+}
+unsigned long xc_domain_get_outstanding_pages(xc_interface *xch)
+{
+    long ret = do_memory_op(xch, XENMEM_get_outstanding_pages, NULL, 0);
+
+    /* Ignore it if the hypervisor does not support the call. */
+    if (ret == -1 && errno == ENOSYS)
+        ret = errno = 0;
+    return ret;
+}
+
 int xc_domain_populate_physmap(xc_interface *xch,
                                uint32_t domid,
                                unsigned long nr_extents,
diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c
index 3b5d777..ab33a7f 100644
--- a/tools/libxc/xc_hvm_build_x86.c
+++ b/tools/libxc/xc_hvm_build_x86.c
@@ -252,6 +252,7 @@ static int setup_guest(xc_interface *xch,
     unsigned long stat_normal_pages = 0, stat_2mb_pages = 0, 
         stat_1gb_pages = 0;
     int pod_mode = 0;
+    int claim_enabled = args->claim_enabled;
 
     if ( nr_pages > target_pages )
         pod_mode = XENMEMF_populate_on_demand;
@@ -329,6 +330,16 @@ static int setup_guest(xc_interface *xch,
         xch, dom, 0xa0, 0, pod_mode, &page_array[0x00]);
     cur_pages = 0xc0;
     stat_normal_pages = 0xc0;
+
+    /* try to claim pages for early warning of insufficient memory available */
+    if ( claim_enabled ) {
+        rc = xc_domain_claim_pages(xch, dom, nr_pages - cur_pages);
+        if ( rc != 0 )
+        {
+            PERROR("Could not allocate memory for HVM guest as we cannot claim memory!");
+            goto error_out;
+        }
+    }
     while ( (rc == 0) && (nr_pages > cur_pages) )
     {
         /* Clip count to maximum 1GB extent. */
@@ -506,12 +517,16 @@ static int setup_guest(xc_interface *xch,
         munmap(page0, PAGE_SIZE);
     }
 
-    free(page_array);
-    return 0;
-
+    rc = 0;
+    goto out;
  error_out:
+    rc = -1;
+ out:
+    /* ensure no unclaimed pages are left unused */
+    xc_domain_claim_pages(xch, dom, 0 /* cancels the claim */);
+
     free(page_array);
-    return -1;
+    return rc;
 }
 
 /* xc_hvm_build:
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index 32122fd..e695456 100644
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -1129,6 +1129,12 @@ int xc_domain_populate_physmap_exact(xc_interface *xch,
                                      unsigned int mem_flags,
                                      xen_pfn_t *extent_start);
 
+int xc_domain_claim_pages(xc_interface *xch,
+                               uint32_t domid,
+                               unsigned long nr_pages);
+
+unsigned long xc_domain_get_outstanding_pages(xc_interface *xch);
+
 int xc_domain_memory_exchange_pages(xc_interface *xch,
                                     int domid,
                                     unsigned long nr_in_extents,
diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h
index 7d4ac33..4714bd2 100644
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -231,6 +231,8 @@ struct xc_hvm_build_args {
 
     /* Extra SMBIOS structures passed to HVMLOADER */
     struct xc_hvm_firmware_module smbios_module;
+    /* Whether to use claim hypercall (1 - enable, 0 - disable). */
+    int claim_enabled;
 };
 
 /**
-- 
1.8.1.4

[PATCH 2/7] xl: Implement XENMEM_claim_pages support via 'claim_mode' global config

[Konrad Rzeszutek Wilk]

The XENMEM_claim_pages hypercall operates per domain and it should be
used system wide. As such this patch introduces a global configuration
option 'claim_mode' that by default is disabled.

If this option is enabled then when a guest is created there will be an
guarantee that there is memory available for the guest. This is an
particularly acute problem on hosts with memory over-provisioned guests
that use tmem and have self-balloon enabled (which is the default option
for them). The self-balloon mechanism can deflate/inflate the balloon
quickly and the amount of free memory (which 'xl info' can show) is stale
the moment it is printed. When claim is enabled a reservation for the
amount of memory ('memory' in guest config) is set, which is then reduced
as the domain's memory is populated and eventually reaches zero.

If the reservation cannot be meet the guest creation fails immediately
instead of taking seconds/minutes (depending on the size of the guest)
while the guest is populated.

Note that to enable tmem type guests, one needs to provide 'tmem' on the
Xen hypervisor argument and as well on the Linux kernel command line.

There are two boolean options:

(0) No claim is made. Memory population during guest creation will be
attempted as normal and may fail due to memory exhaustion.

(1) Normal memory and freeable pool of ephemeral pages (tmem) is used when
calculating whether there is enough memory free to launch a guest.
This guarantees immediate feedback whether the guest can be launched due
to memory exhaustion (which can take a long time to find out if launching
massively huge guests) and in parallel.

[v1: Removed own claim_mode type, using just bool, improved docs, all per
Ian's suggestion]
[v2: Updated the comments]
[v3: Rebase on top 733b9c524dbc2bec318bfc3588ed1652455d30ec (xl: add vif.default.script)]
[v4: Fixed up comments]
[v5: s/global_claim_mode/claim_mode/]
[v6: Ian Jackson's feedback: use libxl_defbool, better comments, etc]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 docs/man/xl.conf.pod.5      | 41 +++++++++++++++++++++++++++++++++++++++++
 tools/examples/xl.conf      |  6 ++++++
 tools/libxl/libxl.h         |  1 -
 tools/libxl/libxl_create.c  |  2 ++
 tools/libxl/libxl_dom.c     |  3 ++-
 tools/libxl/libxl_types.idl |  2 +-
 tools/libxl/xl.c            |  5 +++++
 tools/libxl/xl.h            |  1 +
 tools/libxl/xl_cmdimpl.c    |  2 ++
 9 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
index aaf8da1..c4072aa 100644
--- a/docs/man/xl.conf.pod.5
+++ b/docs/man/xl.conf.pod.5
@@ -115,6 +115,47 @@ Configures the name of the first block device to be used for temporary
 block device allocations by the toolstack.
 The default choice is "xvda".
 
+=item B<claim_mode=BOOLEAN>
+
+If this option is enabled then when a guest is created there will be an
+guarantee that there is memory available for the guest. This is an
+particularly acute problem on hosts with memory over-provisioned guests
+that use tmem and have self-balloon enabled (which is the default
+option). The self-balloon mechanism can deflate/inflate the balloon
+quickly and the amount of free memory (which C<xl info> can show) is
+stale the moment it is printed. When claim is enabled a reservation for
+the amount of memory (see 'memory' in xl.conf(5)) is set, which is then
+reduced as the domain's memory is populated and eventually reaches zero.
+
+If the reservation cannot be meet the guest creation fails immediately
+instead of taking seconds/minutes (depending on the size of the guest)
+while the guest is populated.
+
+Note that to enable tmem type guests, one needs to provide C<tmem> on the
+Xen hypervisor argument and as well on the Linux kernel command line.
+
+Note that the claim call is not attempted if C<superpages> option is
+used in the guest config (see xl.cfg(5)).
+
+Default: C<0>
+
+=over 4
+
+=item C<0>
+
+No claim is made. Memory population during guest creation will be
+attempted as normal and may fail due to memory exhaustion.
+
+=item C<1>
+
+Normal memory and freeable pool of ephemeral pages (tmem) is used when
+calculating whether there is enough memory free to launch a guest.
+This guarantees immediate feedback whether the guest can be launched due
+to memory exhaustion (which can take a long time to find out if launching
+massively huge guests).
+
+=back
+
 =back
 
 =head1 SEE ALSO
diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf
index 50cba2b..9402c3f 100644
--- a/tools/examples/xl.conf
+++ b/tools/examples/xl.conf
@@ -27,3 +27,9 @@
 
 # default bridge device to use with vif-bridge hotplug scripts
 #vif.default.bridge="xenbr0"
+
+# Reserve a claim of memory when launching a guest. This guarantees immediate
+# feedback whether the guest can be launched due to memory exhaustion
+# (which can take a long time to find out if launching huge guests).
+# see xl.conf(5) for details.
+#claim_mode=0
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index d18d22c..e4a4ab2 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -595,7 +595,6 @@ int libxl_wait_for_free_memory(libxl_ctx *ctx, uint32_t domid, uint32_t memory_k
 /* wait for the memory target of a domain to be reached */
 int libxl_wait_for_memory_target(libxl_ctx *ctx, uint32_t domid, int wait_secs);
 
-
 int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass);
 int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, libxl_console_type type);
 /* libxl_primary_console_exec finds the domid and console number
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 30a4507..ae72f21 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -196,6 +196,8 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
     if (b_info->target_memkb == LIBXL_MEMKB_DEFAULT)
         b_info->target_memkb = b_info->max_memkb;
 
+    libxl_defbool_setdefault(&b_info->claim_mode, false);
+
     libxl_defbool_setdefault(&b_info->localtime, false);
 
     libxl_defbool_setdefault(&b_info->disable_migrate, false);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 2dd429f..92a6628 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -371,6 +371,7 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
     dom->console_domid = state->console_domid;
     dom->xenstore_evtchn = state->store_port;
     dom->xenstore_domid = state->store_domid;
+    dom->claim_enabled = libxl_defbool_val(info->claim_mode);
 
     if ( (ret = xc_dom_boot_xen_init(dom, ctx->xch, domid)) != 0 ) {
         LOGE(ERROR, "xc_dom_boot_xen_init failed");
@@ -605,7 +606,7 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
      */
     args.mem_size = (uint64_t)(info->max_memkb - info->video_memkb) << 10;
     args.mem_target = (uint64_t)(info->target_memkb - info->video_memkb) << 10;
-
+    args.claim_enabled = libxl_defbool_val(info->claim_mode);
     if (libxl__domain_firmware(gc, info, &args)) {
         LOG(ERROR, "initializing domain firmware failed");
         goto out;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 6cb6de6..4d8f7cd 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -293,7 +293,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("ioports",          Array(libxl_ioport_range, "num_ioports")),
     ("irqs",             Array(uint32, "num_irqs")),
     ("iomem",            Array(libxl_iomem_range, "num_iomem")),
-
+    ("claim_mode",	     libxl_defbool),
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
                                        ("bios",             libxl_bios_type),
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index 16cd3f3..3c141bf 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -46,6 +46,7 @@ char *default_vifscript = NULL;
 char *default_bridge = NULL;
 char *default_gatewaydev = NULL;
 enum output_format default_output_format = OUTPUT_FORMAT_JSON;
+libxl_defbool claim_mode;
 
 static xentoollog_level minmsglevel = XTL_PROGRESS;
 
@@ -168,6 +169,10 @@ static void parse_global_config(const char *configfile,
     }
     if (!xlu_cfg_get_string (config, "blkdev_start", &buf, 0))
         blkdev_start = strdup(buf);
+
+    libxl_defbool_setdefault(&claim_mode, false);
+    (void)xlu_cfg_get_defbool (config, "claim_mode", &claim_mode, 0);
+
     xlu_cfg_destroy(config);
 }
 
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index b881f92..4c5e5d1 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -145,6 +145,7 @@ int xl_child_pid(xlchildnum); /* returns 0 if child struct is not in use */
 extern int autoballoon;
 extern int run_hotplug_scripts;
 extern int dryrun_only;
+extern libxl_defbool claim_mode;
 extern char *lockfile;
 extern char *default_vifscript;
 extern char *default_bridge;
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 61f7b96..5a0506f 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -757,6 +757,8 @@ static void parse_config_data(const char *config_source,
     if (!xlu_cfg_get_long (config, "maxmem", &l, 0))
         b_info->max_memkb = l * 1024;
 
+    b_info->claim_mode = claim_mode;
+
     if (xlu_cfg_get_string (config, "on_poweroff", &buf, 0))
         buf = "destroy";
     if (!parse_action_on_shutdown(buf, &d_config->on_poweroff)) {
-- 
1.8.1.4

[PATCH 3/7] xl: 'xl info' print outstanding claims if enabled (claim_mode=1 in xl.conf)

[Konrad Rzeszutek Wilk]

This patch provides the value of the currently outstanding pages
claimed for all domains. This is a total global value that influences
the hypervisors' MM system.

When a claim call is done, a reservation for a specific amount of pages
is set and also a global value is incremented. This global value is then
reduced as the domain's memory is populated and eventually reaches zero.
The toolstack (libxc) also sets the domain's claim to zero when the population
of memory has completed as an extra step. Any call to destroy the domain
will also set the domain's claim to zero.

If the reservation cannot be meet the guest creation fails immediately
instead of taking seconds or minutes (depending on the size of the guest)
while the toolstack populates memory.

See patch: "xl: Implement XENMEM_claim_pages support via 'claim_mode'
global config" for details on how it is implemented.

The value fluctuates quite often so the value is stale once it is provided
to the user-space.  However it is useful for diagnostic purposes.

It is only printed when the global "claim_mode" option in xl.conf(5)
is set to enabled (1). The 'man xl' shows the details of this item.

[v1: s/unclaimed/outstanding/]
[v2: Made libxl_get_claiminfo return just MemKB suggested by Ian Campbell]
[v3: Made libxl_get_claininfo return MemMB to conform to the other values printed]
[v4: Improvements suggested by Ian Jackson, also added docs to xl.pod.1]
[v5: Clarify how claims are cancelled, split >72 characters - Ian Jackson]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 docs/man/xl.pod.1        | 10 ++++++++++
 tools/libxl/libxl.c      | 14 ++++++++++++++
 tools/libxl/libxl.h      |  1 +
 tools/libxl/xl_cmdimpl.c | 24 ++++++++++++++++++++++++
 4 files changed, 49 insertions(+)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index a0e298e..d8783e8 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -704,6 +704,7 @@ Sample output looks as follows:
  total_memory           : 6141
  free_memory            : 4274
  free_cpus              : 0
+ outstanding_claims     : 0
  xen_major              : 4
  xen_minor              : 2
  xen_extra              : -unstable
@@ -738,6 +739,15 @@ the feature bits returned by the cpuid command on x86 platforms.
 
 Available memory (in MB) not allocated to Xen, or any other domains.
 
+=item B<outstanding_claims>
+
+When a claim call is done (see L<xl.conf>) a reservation for a specific
+amount of pages is set and also a global value is incremented. This
+global value (outstanding_claims) is then reduced as the domain's memory
+is populated and eventually reaches zero. Most of the time the value will
+be zero, but if you are launching multiple guests, and B<claim_mode> is
+enabled, this value can increase/decrease.
+
 =item B<xen_caps>
 
 The Xen version and architecture.  Architecture values can be one of:
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 572c2c6..230b954 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4057,6 +4057,20 @@ libxl_numainfo *libxl_get_numainfo(libxl_ctx *ctx, int *nr)
     return ret;
 }
 
+uint64_t libxl_get_claiminfo(libxl_ctx *ctx)
+{
+    long l;
+
+    l = xc_domain_get_outstanding_pages(ctx->xch);
+    if (l < 0) {
+        LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_WARNING, l,
+                            "xc_domain_get_outstanding_pages failed.");
+        return ERROR_FAIL;
+    }
+    /* In MB */
+    return (l >> 8);
+}
+
 const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
 {
     union {
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index e4a4ab2..4922313 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -595,6 +595,7 @@ int libxl_wait_for_free_memory(libxl_ctx *ctx, uint32_t domid, uint32_t memory_k
 /* wait for the memory target of a domain to be reached */
 int libxl_wait_for_memory_target(libxl_ctx *ctx, uint32_t domid, int wait_secs);
 
+uint64_t libxl_get_claiminfo(libxl_ctx *ctx);
 int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass);
 int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, libxl_console_type type);
 /* libxl_primary_console_exec finds the domid and console number
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5a0506f..c9b71e6 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4666,6 +4666,29 @@ static void output_topologyinfo(void)
     return;
 }
 
+static void output_claim(void)
+{
+    long l;
+
+    /*
+     * Note that the xl.c (which calls us) has already read from the
+     * global configuration the 'claim_mode' value.
+     */
+    if (!libxl_defbool_val(claim_mode))
+        return;
+
+    l = libxl_get_claiminfo(ctx);
+    if (l < 0) {
+        fprintf(stderr, "libxl_get_claiminfo failed. errno: %d (%s)\n",
+                errno, strerror(errno));
+        return;
+    }
+
+    printf("outstanding_claims     : %ld\n", l);
+
+    return;
+}
+
 static void print_info(int numa)
 {
     output_nodeinfo();
@@ -4676,6 +4699,7 @@ static void print_info(int numa)
         output_topologyinfo();
         output_numainfo();
     }
+    output_claim();
 
     output_xeninfo();
 
-- 
1.8.1.4

[PATCH 4/7] xc: export outstanding_pages value in xc_dominfo structure.

[Konrad Rzeszutek Wilk]

From: Dan Magenheimer <dan.magenheimer@oracle.com>

This patch provides the value of the currently outstanding pages
claimed for a specific domain. This is a value that influences
the global outstanding claims value (See patch: "xl: 'xl info'
print outstanding claims if enabled") returned via
xc_domain_get_outstanding_pages hypercall. This domain value
decrements as the memory is populated for the guest and
eventually reaches zero.

This patch is neccessary for "xl: export 'outstanding_pages' value
from xcinfo" patch.

Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
[v2: s/unclaimed_pages/outstanding_pages/ per Tim's suggestion]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxc/xc_domain.c | 1 +
 tools/libxc/xenctrl.h   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 299c907..1676bd7 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -234,6 +234,7 @@ int xc_domain_getinfo(xc_interface *xch,
 
         info->ssidref  = domctl.u.getdomaininfo.ssidref;
         info->nr_pages = domctl.u.getdomaininfo.tot_pages;
+        info->nr_outstanding_pages = domctl.u.getdomaininfo.outstanding_pages;
         info->nr_shared_pages = domctl.u.getdomaininfo.shr_pages;
         info->nr_paged_pages = domctl.u.getdomaininfo.paged_pages;
         info->max_memkb = domctl.u.getdomaininfo.max_pages << (PAGE_SHIFT-10);
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index e695456..2a4d4df 100644
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -364,6 +364,7 @@ typedef struct xc_dominfo {
                   hvm:1, debugged:1;
     unsigned int  shutdown_reason; /* only meaningful if shutdown==1 */
     unsigned long nr_pages; /* current number, not maximum */
+    unsigned long nr_outstanding_pages;
     unsigned long nr_shared_pages;
     unsigned long nr_paged_pages;
     unsigned long shared_info_frame;
-- 
1.8.1.4

[PATCH 5/7] xl: export 'outstanding_pages' value from xcinfo

[Konrad Rzeszutek Wilk]

This patch provides the value of the currently outstanding pages
claimed for a specific domain. This is a value that influences
the global outstanding claims value (See patch: "xl: 'xl info'
print outstanding claims if enabled") returned via
xc_domain_get_outstanding_pages hypercall. This domain value
decrements as the memory is populated for the guest and
eventually reaches zero.

With this patch it is possible to utilize this field.

Acked-by: Ian Campbell <ian.campbell@citrix.com>
[v2: s/unclaimed/outstanding/ per Tim's suggestion]
[v3: Don't use SXP printout file per Ian's suggestion]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxl/libxl.c         | 1 +
 tools/libxl/libxl_types.idl | 1 +
 2 files changed, 2 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 230b954..8b0e415 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -528,6 +528,7 @@ static void xcinfo2xlinfo(const xc_domaininfo_t *xcinfo,
     else
         xlinfo->shutdown_reason  = ~0;
 
+    xlinfo->outstanding_memkb = PAGE_TO_MEMKB(xcinfo->outstanding_pages);
     xlinfo->current_memkb = PAGE_TO_MEMKB(xcinfo->tot_pages);
     xlinfo->shared_memkb = PAGE_TO_MEMKB(xcinfo->shr_pages);
     xlinfo->paged_memkb = PAGE_TO_MEMKB(xcinfo->paged_pages);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 4d8f7cd..fcb1ecd 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -196,6 +196,7 @@ libxl_dominfo = Struct("dominfo",[
     # Otherwise set to a value guaranteed not to clash with any valid
     # LIBXL_SHUTDOWN_REASON_* constant.
     ("shutdown_reason", libxl_shutdown_reason),
+    ("outstanding_memkb",  MemKB),
     ("current_memkb",   MemKB),
     ("shared_memkb", MemKB),
     ("paged_memkb", MemKB),
-- 
1.8.1.4

[PATCH 6/7] xl: 'xl claims' print outstanding per domain claims

[Konrad Rzeszutek Wilk]

This is similar to "xl: 'xl info' print outstanding claims if enabled
(claim_mode=1 in xl.conf)" which exposes the global claim value.

This patch provides the value of the currently outstanding pages
claimed for each domains. This is per domain value which is added
to the global claim value which influences the hypervisors' MM system.

When a claim call is done, a reservation for a specific amount of pages
is set (and this patch lists said number) and also a global value is
incremented. This global value is then reduced as the domain's memory
is populated and eventually reaches zero.

The toolstack (libxc) also sets the domain's claim to zero when the population
of memory has completed as an extra step. Any call to destroy the domain
will also set the domain's claim to zero.

If the reservation cannot be meet the guest creation fails immediately
instead of taking seconds or minutes (depending on the size of the guest)
while the toolstack populates memory.

See patch: "xl: Implement XENMEM_claim_pages support via 'claim_mode'
global config" for details on how it is implemented.

The value fluctuates quite often so the value is stale once it is provided
to the user-space.  However it is useful for diagnostic purposes.

It is printed irregardless of global "claim_mode" option in xl.conf(5).
That is b/c the user might have enabled, launched a guest, and then
disabled the option - and we should still report the correct outstanding
claim value.  The 'man xl' shows the details of this argument.
The output is close to what 'xl list' looks like:

Name                                        ID   Mem VCPUs      State   Time(s)  Claimed
Domain-0                                     0  2047     4     r-----      19.7     0
OL5                                          2  2048     1     --p---       0.0   847
OL6                                          3  1024     4     r-----       5.9     0
Windows_XP                                   4  2047     1     --p---       0.0  1989

[In which it can be seen that the OL5 guest still has 847MB of claimed
memory (out of the total 2048MB where 1191MB has been allocated to
the guest).]

Please note that the 'Mem' column has the cumulative value of outstanding
claims and the total amount of memory that has been allocated to the guest.

[v1: claims, not claim-list]
[v2: Add outstanding and current memkb in the output list]
[v3: Clairy docs and relax some checks]
[v4: Removed comments about guest config memory being the same as 'Mem']
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 docs/man/xl.pod.1         | 23 +++++++++++++++++++++++
 tools/libxl/libxl.c       |  4 ++--
 tools/libxl/xl.h          |  1 +
 tools/libxl/xl_cmdimpl.c  | 40 ++++++++++++++++++++++++++++++++++++----
 tools/libxl/xl_cmdtable.c |  6 ++++++
 5 files changed, 68 insertions(+), 6 deletions(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index d8783e8..01ecc83 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -780,6 +780,29 @@ explanatory.
 
 Prints the current uptime of the domains running.
 
+=item B<claims>
+
+Prints information about outstanding claims by the guests. This provides
+the outstanding claims and currently populated memory count for the guests.
+These values added up reflect the global outstanding claim value, which
+is provided via the I<info> argument, B<outstanding_claims> value.
+The B<Mem> column has the cumulative value of outstanding claims and
+the total amount of memory that has been right now allocated to the guest.
+
+B<EXAMPLE>
+
+An example format for the list is as follows:
+
+ Name                                        ID   Mem VCPUs      State   Time(s)  Claimed
+ Domain-0                                     0  2047     4     r-----      19.7     0
+ OL5                                          2  2048     1     --p---       0.0   847
+ OL6                                          3  1024     4     r-----       5.9     0
+ Windows_XP                                   4  2047     1     --p---       0.0  1989
+
+In which it can be seen that the OL5 guest still has 847MB of claimed
+memory (out of the total 2048MB where 1191MB has been allocated to
+the guest).
+
 =back
 
 =head1 SCHEDULER SUBCOMMANDS
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 8b0e415..c9905e3 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3865,9 +3865,9 @@ int libxl_wait_for_memory_target(libxl_ctx *ctx, uint32_t domid, int wait_secs)
         rc = libxl_domain_info(ctx, &info, domid);
         if (rc < 0)
             return rc;
-    } while (wait_secs > 0 && info.current_memkb > target_memkb);
+    } while (wait_secs > 0 && (info.current_memkb + info.outstanding_memkb) > target_memkb);
 
-    if (info.current_memkb <= target_memkb)
+    if ((info.current_memkb + info.outstanding_memkb) <= target_memkb)
         rc = 0;
     else
         rc = ERROR_FAIL;
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 4c5e5d1..771b4af 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -83,6 +83,7 @@ int main_vtpmattach(int argc, char **argv);
 int main_vtpmlist(int argc, char **argv);
 int main_vtpmdetach(int argc, char **argv);
 int main_uptime(int argc, char **argv);
+int main_claims(int argc, char **argv);
 int main_tmem_list(int argc, char **argv);
 int main_tmem_freeze(int argc, char **argv);
 int main_tmem_thaw(int argc, char **argv);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index c9b71e6..09b0f41 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3061,7 +3061,8 @@ out:
     }
 }
 
-static void list_domains(int verbose, int context, const libxl_dominfo *info, int nb_domain)
+static void list_domains(int verbose, int context, int claim,
+                         const libxl_dominfo *info, int nb_domain)
 {
     int i;
     static const char shutdown_reason_letters[]= "-rscw";
@@ -3069,6 +3070,7 @@ static void list_domains(int verbose, int context, const libxl_dominfo *info, in
     printf("Name                                        ID   Mem VCPUs\tState\tTime(s)");
     if (verbose) printf("   UUID                            Reason-Code\tSecurity Label");
     if (context && !verbose) printf("   Security Label");
+    if (claim) printf("  Claimed");
     printf("\n");
     for (i = 0; i < nb_domain; i++) {
         char *domname;
@@ -3078,7 +3080,8 @@ static void list_domains(int verbose, int context, const libxl_dominfo *info, in
         printf("%-40s %5d %5lu %5d     %c%c%c%c%c%c  %8.1f",
                 domname,
                 info[i].domid,
-                (unsigned long) (info[i].current_memkb / 1024),
+                (unsigned long) ((info[i].current_memkb +
+                    info[i].outstanding_memkb)/ 1024),
                 info[i].vcpu_online,
                 info[i].running ? 'r' : '-',
                 info[i].blocked ? 'b' : '-',
@@ -3095,6 +3098,8 @@ static void list_domains(int verbose, int context, const libxl_dominfo *info, in
             if (info[i].shutdown) printf(" %8x", shutdown_reason);
             else printf(" %8s", "-");
         }
+        if (claim)
+            printf(" %5lu", (unsigned long)info[i].outstanding_memkb / 1024);
         if (verbose || context) {
             int rc;
             size_t size;
@@ -4029,7 +4034,7 @@ int main_list(int argc, char **argv)
     if (details)
         list_domains_details(info, nb_domain);
     else
-        list_domains(verbose, context, info, nb_domain);
+        list_domains(verbose, context, 0 /* claim */, info, nb_domain);
 
     if (info_free)
         libxl_dominfo_list_free(info, nb_domain);
@@ -4742,7 +4747,8 @@ static void sharing(const libxl_dominfo *info, int nb_domain)
         printf("%-40s %5d %5lu  %5lu\n",
                 domname,
                 info[i].domid,
-                (unsigned long) (info[i].current_memkb / 1024),
+                (unsigned long) ((info[i].current_memkb +
+                    info[i].outstanding_memkb) / 1024),
                 (unsigned long) (info[i].shared_memkb / 1024));
         free(domname);
     }
@@ -5927,6 +5933,32 @@ static char *uptime_to_string(unsigned long uptime, int short_mode)
     return time_string;
 }
 
+int main_claims(int argc, char **argv)
+{
+    libxl_dominfo *info;
+    int opt;
+    int nb_domain;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "claims", 0) {
+        /* No options */
+    }
+
+    if (!libxl_defbool_val(claim_mode))
+        fprintf(stderr, "claim_mode not enabled (see man xl.conf).\n");
+
+    info = libxl_list_domain(ctx, &nb_domain);
+    if (!info) {
+        fprintf(stderr, "libxl_domain_infolist failed.\n");
+        return 1;
+    }
+
+    list_domains(0 /* verbose */, 0 /* context */, 1 /* claim */,
+                 info, nb_domain);
+
+    libxl_dominfo_list_free(info, nb_domain);
+    return 0;
+}
+
 static char *current_time_to_string(time_t now)
 {
     char now_str[100];
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index b4a87ca..00899f5 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -362,6 +362,12 @@ struct cmd_spec cmd_table[] = {
       "Print uptime for all/some domains",
       "[-s] [Domain]",
     },
+    { "claims",
+      &main_claims, 0, 0,
+      "List outstanding claim information about all domains",
+      "",
+      "",
+    },
     { "tmem-list",
       &main_tmem_list, 0, 0,
       "List tmem pools",
-- 
1.8.1.4

[PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value.

[Konrad Rzeszutek Wilk]

Updating to make it clear that free_memory reported by 'xl info'
is influenced by the outstanding claim value. That is the free
memory that will be available to the host once all outstanding
claims have been completed. This modifies the behavior that the
patch titled "xl: 'xl info' print outstanding claims if enabled
(claim_mode=1 in xl.conf)" had - which reported the
outstanding claims and nothing else.

The free_pages as reported by the hypervisor is the currently
available count of pages on the heap. The outstanding pages is
the total amount of pages reserved for guests (so not taken from
the heap yet). As guests are being populated the memory from the
heap shrinks and the outstanding count of pages decreases.
The total memory used for guests increases.

As the available count of pages on the heap and outstanding
claims are intertwined, report the amount of free memory available
to be a combination of that. That is free heap memory minus the
outstanding pages.

We also make some odd choices in reporting. By default we will
only display 'outstanding_claims' if the claim_mode is enabled
in the global configuration file. However, if there are outstanding
claims, we will ignore the claim_mode and report these values.

Suggested-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 docs/man/xl.conf.pod.5   |  2 ++
 docs/man/xl.pod.1        |  8 ++++++--
 tools/libxl/libxl.c      |  4 ++--
 tools/libxl/xl_cmdimpl.c | 45 +++++++++++++++++----------------------------
 4 files changed, 27 insertions(+), 32 deletions(-)

diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
index c4072aa..1229c8a 100644
--- a/docs/man/xl.conf.pod.5
+++ b/docs/man/xl.conf.pod.5
@@ -126,6 +126,8 @@ quickly and the amount of free memory (which C<xl info> can show) is
 stale the moment it is printed. When claim is enabled a reservation for
 the amount of memory (see 'memory' in xl.conf(5)) is set, which is then
 reduced as the domain's memory is populated and eventually reaches zero.
+The free memory in C<xl info> is the combination of the hypervisor's
+free heap memory minus the outstanding claims value.
 
 If the reservation cannot be meet the guest creation fails immediately
 instead of taking seconds/minutes (depending on the size of the guest)
diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 01ecc83..57c6a79 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -737,7 +737,8 @@ the feature bits returned by the cpuid command on x86 platforms.
 
 =item B<free_memory>
 
-Available memory (in MB) not allocated to Xen, or any other domains.
+Available memory (in MB) not allocated to Xen, or any other domains, or
+claimed for domains.
 
 =item B<outstanding_claims>
 
@@ -746,7 +747,10 @@ amount of pages is set and also a global value is incremented. This
 global value (outstanding_claims) is then reduced as the domain's memory
 is populated and eventually reaches zero. Most of the time the value will
 be zero, but if you are launching multiple guests, and B<claim_mode> is
-enabled, this value can increase/decrease.
+enabled, this value can increase/decrease. Note that the value also
+affects the B<free_memory>  - as it will reflect the free memory
+in the hypervisor minus the outstanding pages claimed for guests.
+See xl I<info> B<claims> parameter for detailed listing.
 
 =item B<xen_caps>
 
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index c9905e3..03a9782 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4068,8 +4068,8 @@ uint64_t libxl_get_claiminfo(libxl_ctx *ctx)
                             "xc_domain_get_outstanding_pages failed.");
         return ERROR_FAIL;
     }
-    /* In MB */
-    return (l >> 8);
+    /* In pages */
+    return l;
 }
 
 const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 09b0f41..98ecf67 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4575,12 +4575,21 @@ static void output_physinfo(void)
     unsigned int i;
     libxl_bitmap cpumap;
     int n = 0;
+    long claims = 0;
 
     if (libxl_get_physinfo(ctx, &info) != 0) {
         fprintf(stderr, "libxl_physinfo failed.\n");
         return;
     }
-
+    /*
+     * Don't bother checking "claim_mode" as user might have turned it off
+     * and we have outstanding claims.
+     */
+    if ((claims = libxl_get_claiminfo(ctx)) < 0){
+        fprintf(stderr, "libxl_get_claiminfo failed. errno: %d (%s)\n",
+                errno, strerror(errno));
+        return;
+    }
     printf("nr_cpus                : %d\n", info.nr_cpus);
     printf("max_cpu_id             : %d\n", info.max_cpu_id);
     printf("nr_nodes               : %d\n", info.nr_nodes);
@@ -4600,10 +4609,16 @@ static void output_physinfo(void)
     if (vinfo) {
         i = (1 << 20) / vinfo->pagesize;
         printf("total_memory           : %"PRIu64"\n", info.total_pages / i);
-        printf("free_memory            : %"PRIu64"\n", info.free_pages / i);
+        printf("free_memory            : %"PRIu64"\n", (info.free_pages - claims) / i);
         printf("sharing_freed_memory   : %"PRIu64"\n", info.sharing_freed_pages / i);
         printf("sharing_used_memory    : %"PRIu64"\n", info.sharing_used_frames / i);
     }
+    /*
+     * Only if enabled (claim_mode=1) or there are outstanding claims.
+     */
+    if (libxl_defbool_val(claim_mode) || claims)
+        printf("outstanding_claims     : %ld\n", claims / i);
+
     if (!libxl_get_freecpus(ctx, &cpumap)) {
         libxl_for_each_bit(i, cpumap)
             if (libxl_bitmap_test(&cpumap, i))
@@ -4611,7 +4626,6 @@ static void output_physinfo(void)
         printf("free_cpus              : %d\n", n);
         free(cpumap.map);
     }
-
     libxl_physinfo_dispose(&info);
     return;
 }
@@ -4671,29 +4685,6 @@ static void output_topologyinfo(void)
     return;
 }
 
-static void output_claim(void)
-{
-    long l;
-
-    /*
-     * Note that the xl.c (which calls us) has already read from the
-     * global configuration the 'claim_mode' value.
-     */
-    if (!libxl_defbool_val(claim_mode))
-        return;
-
-    l = libxl_get_claiminfo(ctx);
-    if (l < 0) {
-        fprintf(stderr, "libxl_get_claiminfo failed. errno: %d (%s)\n",
-                errno, strerror(errno));
-        return;
-    }
-
-    printf("outstanding_claims     : %ld\n", l);
-
-    return;
-}
-
 static void print_info(int numa)
 {
     output_nodeinfo();
@@ -4704,8 +4695,6 @@ static void print_info(int numa)
         output_topologyinfo();
         output_numainfo();
     }
-    output_claim();
-
     output_xeninfo();
 
     printf("xend_config_format     : 4\n");
-- 
1.8.1.4

Re: [PATCH 6/7] xl: 'xl claims' print outstanding per domain claims

[Ian Jackson]

Konrad Rzeszutek Wilk writes ("[PATCH 6/7] xl: 'xl claims' print outstanding per domain claims"):
> This is similar to "xl: 'xl info' print outstanding claims if enabled
> (claim_mode=1 in xl.conf)" which exposes the global claim value.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Re: [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value.

[Ian Jackson]

Konrad Rzeszutek Wilk writes ("[PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value."):
> Updating to make it clear that free_memory reported by 'xl info'
> is influenced by the outstanding claim value. [...]

>      if (libxl_get_physinfo(ctx, &info) != 0) {
>          fprintf(stderr, "libxl_physinfo failed.\n");
>          return;
>      }
> -
> +    /*
> +     * Don't bother checking "claim_mode" as user might have turned it off
> +     * and we have outstanding claims.
> +     */
> +    if ((claims = libxl_get_claiminfo(ctx)) < 0){
> +        fprintf(stderr, "libxl_get_claiminfo failed. errno: %d (%s)\n",
> +                errno, strerror(errno));
> +        return;
> +    }
...
> +        printf("free_memory            : %"PRIu64"\n", (info.free_pages - claims) / i);

This has a race, I think.

Ian.

Re: [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value.

[Dario Faggioli]

On ven, 2013-04-12 at 16:56 -0400, Konrad Rzeszutek Wilk wrote:

> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 09b0f41..98ecf67 100644
>
> [..]
>
> @@ -4611,7 +4626,6 @@ static void output_physinfo(void)
>          printf("free_cpus              : %d\n", n);
>          free(cpumap.map);
>      }
> -
>      libxl_physinfo_dispose(&info);
>      return;
>  }
>
What did this poor, empty, line do wrong, for deserving being
"killed"? :-P

Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Re: [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value.

[Ian Jackson]

Ian Jackson writes ("Re: [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value."):
> Konrad Rzeszutek Wilk writes ("[PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value."):
> > Updating to make it clear that free_memory reported by 'xl info'
> > is influenced by the outstanding claim value. [...]
> 
> >      if (libxl_get_physinfo(ctx, &info) != 0) {
> >          fprintf(stderr, "libxl_physinfo failed.\n");
> >          return;
> >      }
> > -
> > +    /*
> > +     * Don't bother checking "claim_mode" as user might have turned it off
> > +     * and we have outstanding claims.
> > +     */
> > +    if ((claims = libxl_get_claiminfo(ctx)) < 0){
> > +        fprintf(stderr, "libxl_get_claiminfo failed. errno: %d (%s)\n",
> > +                errno, strerror(errno));
> > +        return;
> > +    }
> ...
> > +        printf("free_memory            : %"PRIu64"\n", (info.free_pages - claims) / i);
> 
> This has a race, I think.

I have checked this and the race is in the hypercall API.  The
hypercall API has already been checked in.  So, under the
circumstances, for this patch:

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>


However, there is a release-critical bug here.  The following things
need to be changed:

 * We need a race-free version of the hypercall API.
 * We need a race-free version of the xc API.
 * We need a race-free version of the libxl API.

I think this is a release-critical bug because fixing it involves an
API change at all these 3 levels.  We don't want to release 4.3 with
a broken API as that will complicate fixing this bug.

George, can you please add this to your tracking list ?

Having said all that, George, am I OK from a freeze POV to pull
Konrad's series into staging ?

Ian.

Re: [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value.

[konrad wilk]

On 4/15/2013 11:03 AM, Dario Faggioli wrote:
> On ven, 2013-04-12 at 16:56 -0400, Konrad Rzeszutek Wilk wrote:
>
>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
>> index 09b0f41..98ecf67 100644
>>
>> [..]
>>
>> @@ -4611,7 +4626,6 @@ static void output_physinfo(void)
>>           printf("free_cpus              : %d\n", n);
>>           free(cpumap.map);
>>       }
>> -
>>       libxl_physinfo_dispose(&info);
>>       return;
>>   }
>>
> What did this poor, empty, line do wrong, for deserving being
> "killed"? :-P

It initially offended me by being so nicely placed. But your effective 
plea for its resurrection convinced me that it should be brought back. 
The upcoming patch shall have it restored

Re: [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value.

[konrad wilk]

On 4/15/2013 12:50 PM, Ian Jackson wrote:
> Ian Jackson writes ("Re: [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value."):
>> Konrad Rzeszutek Wilk writes ("[PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value."):
>>> Updating to make it clear that free_memory reported by 'xl info'
>>> is influenced by the outstanding claim value. [...]
>>>       if (libxl_get_physinfo(ctx, &info) != 0) {
>>>           fprintf(stderr, "libxl_physinfo failed.\n");
>>>           return;
>>>       }
>>> -
>>> +    /*
>>> +     * Don't bother checking "claim_mode" as user might have turned it off
>>> +     * and we have outstanding claims.
>>> +     */
>>> +    if ((claims = libxl_get_claiminfo(ctx)) < 0){
>>> +        fprintf(stderr, "libxl_get_claiminfo failed. errno: %d (%s)\n",
>>> +                errno, strerror(errno));
>>> +        return;
>>> +    }
>> ...
>>> +        printf("free_memory            : %"PRIu64"\n", (info.free_pages - claims) / i);
>> This has a race, I think.
> I have checked this and the race is in the hypercall API.  The
> hypercall API has already been checked in.  So, under the
> circumstances, for this patch:
>
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
>

Great! Thanks.
> However, there is a release-critical bug here.  The following things
> need to be changed:
>
>   * We need a race-free version of the hypercall API.
>   * We need a race-free version of the xc API.
>   * We need a race-free version of the libxl API.
>
> I think this is a release-critical bug because fixing it involves an
> API change at all these 3 levels.  We don't want to release 4.3 with
> a broken API as that will complicate fixing this bug.

Right. Let me enumerate some ideas for fixing this on a different thread 
(if we have the same race in mind that you spotted).
>
> George, can you please add this to your tracking list ?
>
> Having said all that, George, am I OK from a freeze POV to pull
> Konrad's series into staging ?

Would you like me to rebase it once more with the s/def_bool/int/ and 
the resurrection of a line change? I can repost just against the 
offending patch?

I might be a bit late with doing it (Tuesday night) since I am traveling 
today.
>
> Ian.

Re: [PATCH v16] claim and its friends for allocating multiple self-ballooning guests.

[Ian Jackson]

Konrad Rzeszutek Wilk writes ("[PATCH v16] claim and its friends for allocating multiple self-ballooning guests."):
> Note that some of the patches could probably be squashed. In the interest
> of sanity and patience of Ian I choose to not do that - so that he can
> be assured that the ones he Acked have not changed since the last posting.

As discussed, I have rebased this to staging and pushed it.

George, did you put the interface race on your 4.3 tracking list ?

Ian.

Re: [PATCH v16] claim and its friends for allocating multiple self-ballooning guests.

[George Dunlap]

On 16/04/13 16:25, Ian Jackson wrote:
> Konrad Rzeszutek Wilk writes ("[PATCH v16] claim and its friends for allocating multiple self-ballooning guests."):
>> Note that some of the patches could probably be squashed. In the interest
>> of sanity and patience of Ian I choose to not do that - so that he can
>> be assured that the ones he Acked have not changed since the last posting.
> As discussed, I have rebased this to staging and pushed it.
>
> George, did you put the interface race on your 4.3 tracking list ?

Yes:

* Race condition in claim hypercall
   owner: Ian Jackson, Konrad Wilk

  -George

Is: [GIT PULL) (claim.v17) against Xen 4.3-rc0. Was:Re: [PATCH v16] claim and its friends for allocating multiple self-ballooning guests.

[Konrad Rzeszutek Wilk]

On Fri, Apr 12, 2013 at 04:56:14PM -0400, Konrad Rzeszutek Wilk wrote:

I've updated a new version (v17), to be:
 - Added 'Acked-by' on the last two patches
 - Restored the inadvertent drive-by-line-killing.

> Changes since v15:
>  - Added 'Acked-by: Ian' on many patches.
>  - Fixed 'xl claims' call to be smarter about reporting information (reworked
>    a patch).
>  - Fixed 'xl info', free_memory: to also take into account outstanding claims
>    (new patch)
.. snip.

> These patches are also visible at:
> 
>   git://xenbits.xen.org/people/konradwilk/xen.git claim.v16

And the new version is:

   git://xenbits.xen.org/people/konradwilk/xen.git claim.v17

All patches are Acked-by. There are two bugs that need to be fixed
before Xen 4.3 comes out:

 1). Ian Jacksons' observation about race:
	 * We need a race-free version of the hypercall API.
	 * We need a race-free version of the xc API.
	 * We need a race-free version of the libxl API.

 2). defbool vs int and the 'claim_mode' usage.


 docs/man/xl.conf.pod.5         | 43 +++++++++++++++++++++++++++++
 docs/man/xl.pod.1              | 39 +++++++++++++++++++++++++-
 tools/examples/xl.conf         |  6 ++++
 tools/libxc/xc_dom.h           |  1 +
 tools/libxc/xc_dom_x86.c       | 12 ++++++++
 tools/libxc/xc_domain.c        | 31 +++++++++++++++++++++
 tools/libxc/xc_hvm_build_x86.c | 23 +++++++++++++---
 tools/libxc/xenctrl.h          |  7 +++++
 tools/libxc/xenguest.h         |  2 ++
 tools/libxl/libxl.c            | 19 +++++++++++--
 tools/libxl/libxl.h            |  2 +-
 tools/libxl/libxl_create.c     |  2 ++
 tools/libxl/libxl_dom.c        |  3 +-
 tools/libxl/libxl_types.idl    |  3 +-
 tools/libxl/xl.c               |  5 ++++
 tools/libxl/xl.h               |  2 ++
 tools/libxl/xl_cmdimpl.c       | 62 +++++++++++++++++++++++++++++++++++++-----
 tools/libxl/xl_cmdtable.c      |  6 ++++
 18 files changed, 251 insertions(+), 17 deletions(-)

Dan Magenheimer (2):
      xc: use XENMEM_claim_pages hypercall during guest creation.
      xc: export outstanding_pages value in xc_dominfo structure.

Konrad Rzeszutek Wilk (5):
      xl: Implement XENMEM_claim_pages support via 'claim_mode' global config
      xl: 'xl info' print outstanding claims if enabled (claim_mode=1 in xl.conf)
      xl: export 'outstanding_pages' value from xcinfo
      xl: 'xl claims' print outstanding per domain claims
      xl: Fix 'free_memory' to include outstanding_claims value.

Re: [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value.

[Ian Jackson]

konrad wilk writes ("Re: [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value."):
> On 4/15/2013 12:50 PM, Ian Jackson wrote:
> > I have checked this and the race is in the hypercall API.  The
> > hypercall API has already been checked in.  So, under the
> > circumstances, for this patch:
...
> Right. Let me enumerate some ideas for fixing this on a different thread 
> (if we have the same race in mind that you spotted).

The race I'm thinking of is this:

When we display the effective amount of free memory (in "xl info"
etc.), we calculate it as
  physinfo->free_pages - xc_domain_get_outstanding_pages()

But these two values have been retrieved at different times.  So while
a domain is being built and memory moves from "free but claimed" to
"in use", the free memory visible via the libxl API will sometimes be
"wrong".

This may seem like a minor point, but it will show up as occasional
glitches in automatic monitoring and graphing systems; it might even
trigger spurious nagios alerts in higher layers etc.  If that was all
there was to it I wouldn't regard it as a release-critical bug - a
race like that would be annoying but could be fixed later.

However, the race is baked into the hypercall API/ABI which we want to
keep relatively stable at least in stable releases.

I think the right answer is probably simply to move the total claim
value into the physinfo struct, and do away with the separate
XENMEM_get_outstanding_pages memory op hypercall.  Do you agree ?

Thanks,
Ian.

Re: [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value.

[Tim Deegan]

At 16:33 +0100 on 16 Apr (1366129996), Ian Jackson wrote:
> konrad wilk writes ("Re: [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value."):
> > On 4/15/2013 12:50 PM, Ian Jackson wrote:
> > > I have checked this and the race is in the hypercall API.  The
> > > hypercall API has already been checked in.  So, under the
> > > circumstances, for this patch:
> ...
> > Right. Let me enumerate some ideas for fixing this on a different thread 
> > (if we have the same race in mind that you spotted).
> 
> The race I'm thinking of is this:
> 
> When we display the effective amount of free memory (in "xl info"
> etc.), we calculate it as
>   physinfo->free_pages - xc_domain_get_outstanding_pages()
> 
> But these two values have been retrieved at different times.  So while
> a domain is being built and memory moves from "free but claimed" to
> "in use", the free memory visible via the libxl API will sometimes be
> "wrong".
> 
> This may seem like a minor point, but it will show up as occasional
> glitches in automatic monitoring and graphing systems; it might even
> trigger spurious nagios alerts in higher layers etc.  If that was all
> there was to it I wouldn't regard it as a release-critical bug - a
> race like that would be annoying but could be fixed later.
> 
> However, the race is baked into the hypercall API/ABI which we want to
> keep relatively stable at least in stable releases.
> 
> I think the right answer is probably simply to move the total claim
> value into the physinfo struct, and do away with the separate
> XENMEM_get_outstanding_pages memory op hypercall.  Do you agree ?

FWIW, I think this is a good idea.  You might have to be a little
careful on the hypervisor side to make sure the two values are actually
a matched pair (say by taking the page allocator lock).

Tim.

Re: [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value.

[Konrad Rzeszutek Wilk]

On Tue, Apr 16, 2013 at 05:45:20PM +0100, Tim Deegan wrote:
> At 16:33 +0100 on 16 Apr (1366129996), Ian Jackson wrote:
> > konrad wilk writes ("Re: [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value."):
> > > On 4/15/2013 12:50 PM, Ian Jackson wrote:
> > > > I have checked this and the race is in the hypercall API.  The
> > > > hypercall API has already been checked in.  So, under the
> > > > circumstances, for this patch:
> > ...
> > > Right. Let me enumerate some ideas for fixing this on a different thread 
> > > (if we have the same race in mind that you spotted).
> > 
> > The race I'm thinking of is this:
> > 
> > When we display the effective amount of free memory (in "xl info"
> > etc.), we calculate it as
> >   physinfo->free_pages - xc_domain_get_outstanding_pages()
> > 
> > But these two values have been retrieved at different times.  So while
> > a domain is being built and memory moves from "free but claimed" to
> > "in use", the free memory visible via the libxl API will sometimes be
> > "wrong".
> > 
> > This may seem like a minor point, but it will show up as occasional
> > glitches in automatic monitoring and graphing systems; it might even
> > trigger spurious nagios alerts in higher layers etc.  If that was all
> > there was to it I wouldn't regard it as a release-critical bug - a
> > race like that would be annoying but could be fixed later.
> > 
> > However, the race is baked into the hypercall API/ABI which we want to
> > keep relatively stable at least in stable releases.
> > 
> > I think the right answer is probably simply to move the total claim
> > value into the physinfo struct, and do away with the separate
> > XENMEM_get_outstanding_pages memory op hypercall.  Do you agree ?
> 
> FWIW, I think this is a good idea.  You might have to be a little
> careful on the hypervisor side to make sure the two values are actually
> a matched pair (say by taking the page allocator lock).

<nods> Going to prep a patch for that. I might not have it ready this week
as Linus merge window is immienient and I need to queue up patches (And test).

And then right after that I am out for a week.

But after that - I will have the patch ready.
> 
> Tim.