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

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

[Konrad Rzeszutek Wilk]

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.

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

 docs/man/xl.conf.pod.5         | 41 ++++++++++++++++++++++++++++
 docs/man/xl.pod.1              | 27 +++++++++++++++++++
 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            | 15 +++++++++++
 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       | 61 ++++++++++++++++++++++++++++++++++++++++--
 tools/libxl/xl_cmdtable.c      |  6 +++++
 18 files changed, 240 insertions(+), 9 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 (4):
      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 if enabled (claim_mode=1 in xl.conf)

[PATCH 1/6] 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>
---
 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/6] 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>
---
 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/6] 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>
---
 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/6] 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>
---
 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/6] 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>
---
 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/6] xl: 'xl claims' print outstanding per domain claims if enabled (claim_mode=1 in xl.conf)

[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 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.
The output is close to what 'xl list' looks like:

Name                                        ID   Mem VCPUs      State   Time(s)  Claim
Domain-0                                     0  2048     4     r-----      15.7     0
OL5                                          2   321     1     --p---       0.0  1717
OL6                                          3   217     1     --p---       0.0   797

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 docs/man/xl.pod.1         | 17 +++++++++++++++++
 tools/libxl/xl.h          |  1 +
 tools/libxl/xl_cmdimpl.c  | 35 +++++++++++++++++++++++++++++++++--
 tools/libxl/xl_cmdtable.c |  6 ++++++
 4 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index d8783e8..18415b1 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -780,6 +780,23 @@ explanatory.
 
 Prints the current uptime of the domains running.
 
+=item B<claim-list>
+
+Prints information about outstanding claims by the guests. This provides the
+per-domain outstanding claims and memory allocated to the guests. These
+values added up reflect the global outstanding claim information provided via
+the I<info> argument, B<outstanding_claims> value.
+
+B<EXAMPLE>
+
+An example format for the list is as follows:
+
+ Name                                        ID   Mem VCPUs      State   Time(s)  Claim
+ Domain-0                                     0  2047     4     r-----      19.7     0
+ OL5                                          2  1191     1     --p---       0.0   847
+ OL6                                          3  1024     4     r-----       5.9     0
+ Windows_XP                                   4    49     1     --p---       0.0  1989
+
 =back
 
 =head1 SCHEDULER SUBCOMMANDS
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..f702aaf 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("  Claim");
     printf("\n");
     for (i = 0; i < nb_domain; i++) {
         char *domname;
@@ -3095,6 +3097,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 +4033,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);
@@ -5927,6 +5931,33 @@ 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");
+        return 1;
+    }
+    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

Re: [PATCH 6/6] xl: 'xl claims' print outstanding per domain claims if enabled (claim_mode=1 in xl.conf)

[Konrad Rzeszutek Wilk]

. snip..
> diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
> index d8783e8..18415b1 100644
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1
> @@ -780,6 +780,23 @@ explanatory.
>  
>  Prints the current uptime of the domains running.
>  
> +=item B<claim-list>

<Grumble grumble>

Here is an updated one with B<claims> instead of <claim-list>

The update git branch is claim.v15.1


>From 338acbc61b8e617249505f74c35e79bbc332ea0a Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Wed, 10 Apr 2013 14:28:05 -0400
Subject: [PATCH] xl: 'xl claims' print outstanding per domain claims if
 enabled (claim_mode=1 in xl.conf)

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 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.
The output is close to what 'xl list' looks like:

Name                                        ID   Mem VCPUs      State   Time(s)  Claim
Domain-0                                     0  2048     4     r-----      15.7     0
OL5                                          2   321     1     --p---       0.0  1717
OL6                                          3   217     1     --p---       0.0   797

[v1: claims, not claim-list]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 docs/man/xl.pod.1         | 17 +++++++++++++++++
 tools/libxl/xl.h          |  1 +
 tools/libxl/xl_cmdimpl.c  | 35 +++++++++++++++++++++++++++++++++--
 tools/libxl/xl_cmdtable.c |  6 ++++++
 4 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index d8783e8..60e4a86 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -780,6 +780,23 @@ 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.
+
+B<EXAMPLE>
+
+An example format for the list is as follows:
+
+ Name                                        ID   Mem VCPUs      State   Time(s)  Claim
+ Domain-0                                     0  2047     4     r-----      19.7     0
+ OL5                                          2  1191     1     --p---       0.0   847
+ OL6                                          3  1024     4     r-----       5.9     0
+ Windows_XP                                   4    49     1     --p---       0.0  1989
+
 =back
 
 =head1 SCHEDULER SUBCOMMANDS
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..f702aaf 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("  Claim");
     printf("\n");
     for (i = 0; i < nb_domain; i++) {
         char *domname;
@@ -3095,6 +3097,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 +4033,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);
@@ -5927,6 +5931,33 @@ 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");
+        return 1;
+    }
+    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

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

[George Dunlap]

On 10/04/13 20:59, Konrad Rzeszutek Wilk wrote:
> 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.

Do any of these patches need an ACK from me in particular?

I've had a quick look through them and at a high level they seem fine; 
let me know (anyone) if you need something more.

  -George

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

[Konrad Rzeszutek Wilk]

On Fri, Apr 12, 2013 at 11:55:38AM +0100, George Dunlap wrote:
> On 10/04/13 20:59, Konrad Rzeszutek Wilk wrote:
> >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.
> 
> Do any of these patches need an ACK from me in particular?
> 

No, but Ian Jackson at some point CC-ed you on them so I figured I might 
as well continue with that.

> I've had a quick look through them and at a high level they seem
> fine; let me know (anyone) if you need something more.

I am just waiting for either Ian's to Ack it and stick it in the tree.

> 
>  -George
> 

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

[Ian Jackson]

Konrad Rzeszutek Wilk writes ("[PATCH 1/6] xc: use XENMEM_claim_pages hypercall during guest creation."):
> 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>

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

[Ian Jackson]

Konrad Rzeszutek Wilk writes ("[PATCH 2/6] xl: Implement XENMEM_claim_pages support via 'claim_mode' global config"):
> 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.

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

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

[Ian Jackson]

Konrad Rzeszutek Wilk writes ("[PATCH 3/6] xl: 'xl info' print outstanding claims if enabled (claim_mode=1 in xl.conf)"):
> 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.

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

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

[Ian Jackson]

Konrad Rzeszutek Wilk writes ("[PATCH 4/6] xc: export outstanding_pages value in xc_dominfo structure."):
> 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.

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

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

[Ian Jackson]

Konrad Rzeszutek Wilk writes ("[PATCH 5/6] xl: export 'outstanding_pages' value from xcinfo"):
> 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.

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

Re: [PATCH 6/6] xl: 'xl claims' print outstanding per domain claims if enabled (claim_mode=1 in xl.conf)

[Ian Jackson]

Konrad Rzeszutek Wilk writes ("[PATCH 6/6] xl: 'xl claims' print outstanding per domain claims if enabled (claim_mode=1 in xl.conf)"):
> This is similar to "xl: 'xl info' print outstanding claims if enabled
> (claim_mode=1 in xl.conf)" which exposes the global claim value.
...
> Name                                        ID   Mem VCPUs      State   Time(s)  Claim
> Domain-0                                     0  2048     4     r-----      15.7     0
> OL5                                          2   321     1     --p---       0.0  1717
> OL6                                          3   217     1     --p---       0.0   797

We discussed some of this yesterday on IRC.

My understanding from that conversation was that the outstanding claim
was included in the report of the memory used by the guest.

That's not consistent with this example, and nothing in your
documentation explains this.  I haven't checked the hypervisor code so
it may be that this is a docs problem.

Can you confirm that "Mem" as reported by "xl list" _includes_
outstanding claims ?  And please resend with a fix to (a) the example
and (b) the relevant parts of the docs ?

Ian.

Re: [PATCH 6/6] xl: 'xl claims' print outstanding per domain claims if enabled (claim_mode=1 in xl.conf)

[Konrad Rzeszutek Wilk]

On Fri, Apr 12, 2013 at 04:24:43PM +0100, Ian Jackson wrote:
> Konrad Rzeszutek Wilk writes ("[PATCH 6/6] xl: 'xl claims' print outstanding per domain claims if enabled (claim_mode=1 in xl.conf)"):
> > This is similar to "xl: 'xl info' print outstanding claims if enabled
> > (claim_mode=1 in xl.conf)" which exposes the global claim value.
> ...
> > Name                                        ID   Mem VCPUs      State   Time(s)  Claim
> > Domain-0                                     0  2048     4     r-----      15.7     0
> > OL5                                          2   321     1     --p---       0.0  1717
> > OL6                                          3   217     1     --p---       0.0   797
> 
> We discussed some of this yesterday on IRC.
> 
> My understanding from that conversation was that the outstanding claim
> was included in the report of the memory used by the guest.
> 
> That's not consistent with this example, and nothing in your
> documentation explains this.  I haven't checked the hypervisor code so
> it may be that this is a docs problem.
> 
> Can you confirm that "Mem" as reported by "xl list" _includes_
> outstanding claims ?  And please resend with a fix to (a) the example
> and (b) the relevant parts of the docs ?

Please see inline patch:

commit c75cb1dc35f5936238be0a6d49517b34a867880a
Author: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date:   Wed Apr 10 14:28:05 2013 -0400

    xl: 'xl claims' print outstanding per domain claims if enabled (claim_mode=1 in xl.conf)
    
    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 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.
    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 right now allocated
    to the guest. The value equals the "memory" in the guest config
    (see xl.conf manpage).
    
    [v1: claims, not claim-list]
    [v2: Add outstanding and current memkb in the output list]
    Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index c2296ef..a0c14c8 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -280,7 +280,8 @@ An example format for the list is as follows:
 
 Name is the name of the domain.  ID the numeric domain id.  Mem is the
 desired amount of memory to allocate to the domain (although it may
-not be the currently allocated amount).  VCPUs is the number of
+not be the currently allocated amount - unless B<claim_mode> in xl.cfg has
+been enabled, see B<claims> below).  VCPUs is the number of
 virtual CPUs allocated to the domain.  State is the run state (see
 below).  Time is the total run time of the domain as accounted for by
 Xen.
@@ -782,6 +783,30 @@ 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.
+The value equals the I<memory> in the guest config (see xl.conf manpage).
+
+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..80aee7f 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,33 @@ 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");
+        return 1;
+    }
+    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",

Re: [PATCH 6/6] xl: 'xl claims' print outstanding per domain claims if enabled (claim_mode=1 in xl.conf)

[Ian Jackson]

Konrad Rzeszutek Wilk writes ("Re: [PATCH 6/6] xl: 'xl claims' print outstanding per domain claims if enabled (claim_mode=1 in xl.conf)"):
...
> Please see inline patch:

Thanks.

I think this is mostly correct now.  I have a few quibbles about the
docs.

It occurs to me that maybe some of these hunks ought to be folded into
earlier patches in your series, but I'll leave the decision about that
to you.

>     [v1: claims, not claim-list]
>     [v2: Add outstanding and current memkb in the output list]
>     Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
> index c2296ef..a0c14c8 100644
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1
> @@ -280,7 +280,8 @@ An example format for the list is as follows:
>  
>  Name is the name of the domain.  ID the numeric domain id.  Mem is the
>  desired amount of memory to allocate to the domain (although it may
> -not be the currently allocated amount).  VCPUs is the number of
> +not be the currently allocated amount - unless B<claim_mode> in xl.cfg has
> +been enabled, see B<claims> below).  VCPUs is the number of

I'm confused now.

Firstly I'm not sure why existing text says "it may not be the
currently allocated amount".  If it comes from current_memkb then
surely it is exactly the currently allocated amount.

Secondly I don't see how claim mode makes any difference.

> +The value equals the I<memory> in the guest config (see xl.conf manpage).

I'm not sure this is true.  For example, if the guest has been asked
to change its balloon but has not yet reached its target.

> +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).

Great.

> @@ -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);

Was this in the previous version ?  It looks right to me, anyway.

> @@ -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),

Right, I think this is correct.

> @@ -5927,6 +5933,33 @@ static char *uptime_to_string(unsigned long uptime, int short_mode)
...
> +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");
> +        return 1;
> +    }

I think this should be a warning, not an error.  If you turn claim
mode off in the config file, you should still be able to list any
existing claims in previously-created domains.

Thanks,
Ian.

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

[Ian Jackson]

Konrad Rzeszutek Wilk writes ("[PATCH 2/6] xl: Implement XENMEM_claim_pages support via 'claim_mode' global config"):
> 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);

Sorry, I just spotted this.  I think the libxl_defbool_setdefault
shouldn't be there.  The defbool should be initialised to "default",
which can be done by setting it to 0, as per:

  * To allow users of the library to naively select all defaults this
  * state is represented as 0. False is < 0 and True is > 0.

in libxl.h.  And since it's a variable of static duration the C
implementation will initialise it to 0.  So just deleting the
setdefault is right.

The result is that the default is set in libxl, only.

Ian.

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

[Ian Jackson]

Konrad Rzeszutek Wilk writes ("Re: [PATCH v15] claim and its friends for allocating multiple self-ballooning guests."):
> I am just waiting for either Ian's to Ack it and stick it in the tree.

This series is very close.  I'm picking tiny nits.  In the event it
doesn't make feature code freeze I'd like to propose a freeze
exception for it.  The hypervisor side is in tree already and the code
has been floating around on the list for a long time.  The tools parts
are pretty obvious and getting the API design right is what has been
taking the extra time.

Ian.

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

[Ian Jackson]

Ian Jackson writes ("Re: [PATCH 2/6] xl: Implement XENMEM_claim_pages support via 'claim_mode' global config"):
> Sorry, I just spotted this.  I think the libxl_defbool_setdefault
> shouldn't be there.  The defbool should be initialised to "default",
> which can be done by setting it to 0, as per:
> 
>   * To allow users of the library to naively select all defaults this
>   * state is represented as 0. False is < 0 and True is > 0.
> 
> in libxl.h.  And since it's a variable of static duration the C
> implementation will initialise it to 0.  So just deleting the
> setdefault is right.
> 
> The result is that the default is set in libxl, only.

Konrad points out that without this, xl can't easily find out whether
the claim mode is enabled or not.

Options are:

1. Leave it as it is, default set in libxl but overridden by
   separate setting in xl (perhaps we should add a comment).
   Tolerable.

2. Move the default setting out of libxl entirely, so callers
   must pass 0 or 1.  (I don't approve of this; we might want
   to change the behaviour of naive toolstacks in the future.)

3. Provide a new interface to libxl which allows the claim
   default to be retrieved.  Palaver.     

4. Have xl operations which need to know the default claim mode
   set up a dummy domain creation config, ask libxl to set the
   defaults in it, and then read out the value.  Very ugly.

Of these I prefer 1.  Opinions ?  Whatever we do needs to be in 4.3.

Ian.

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

[Ian Jackson]

Ian Jackson writes ("Re: [PATCH 2/6] xl: Implement XENMEM_claim_pages support via 'claim_mode' global config"):
> Of these I prefer 1.  Opinions ?  Whatever we do needs to be in 4.3.

I should clarify that my ack on 2/6 as-is stands.  We can put this in
and fix it up later.

Ian.

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

[Konrad Rzeszutek Wilk]

> Of these I prefer 1.  Opinions ?  Whatever we do needs to be in 4.3.

There is also option 5. Define a new macro:

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 4922313..4a6ee76 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -359,6 +359,11 @@ typedef struct {
     int val;
 } libxl_defbool;
 
+#define DEFINE_BOOL(name, _val) \
+    libxl_defbool name = { .val = _val }
+#define DEFINE_FALSE_BOOL(name) DEFINE_BOOL(name, LIBXL__DEFBOOL_FALSE)
+#define DEFINE_TRUE_BOOL(name) DEFINE_BOOL(name, LIBXL__DEFBOOL_TRUE)
+
 void libxl_defbool_set(libxl_defbool *db, bool b);
 /* Resets to default */
 void libxl_defbool_unset(libxl_defbool *db);


And use DEFINE_FALSE_BOOL(claim_mode) in the xl.h file.

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

[Konrad Rzeszutek Wilk]

On Fri, Apr 12, 2013 at 03:51:04PM -0400, Konrad Rzeszutek Wilk wrote:
> > Of these I prefer 1.  Opinions ?  Whatever we do needs to be in 4.3.
> 
> There is also option 5. Define a new macro:
> 
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 4922313..4a6ee76 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -359,6 +359,11 @@ typedef struct {
>      int val;
>  } libxl_defbool;
>  
> +#define DEFINE_BOOL(name, _val) \
> +    libxl_defbool name = { .val = _val }
> +#define DEFINE_FALSE_BOOL(name) DEFINE_BOOL(name, LIBXL__DEFBOOL_FALSE)
> +#define DEFINE_TRUE_BOOL(name) DEFINE_BOOL(name, LIBXL__DEFBOOL_TRUE)
> +
>  void libxl_defbool_set(libxl_defbool *db, bool b);
>  /* Resets to default */
>  void libxl_defbool_unset(libxl_defbool *db);
> 
> 
> And use DEFINE_FALSE_BOOL(claim_mode) in the xl.h file.
> 

Correction (But now that I look at it, it is not that much nicer as you
end up with LIBXL__DEFBOOL in the header file. <sigh>


diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 03a9782..436b60c 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -222,10 +222,6 @@ void libxl_key_value_list_dispose(libxl_key_value_list *pkvl)
     free(kvl);
 }
 
-#define LIBXL__DEFBOOL_DEFAULT (0)
-#define LIBXL__DEFBOOL_FALSE (-1)
-#define LIBXL__DEFBOOL_TRUE (1)
-
 void libxl_defbool_set(libxl_defbool *db, bool b)
 {
     db->val = b ? LIBXL__DEFBOOL_TRUE : LIBXL__DEFBOOL_FALSE;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 4922313..308a315 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -359,6 +359,15 @@ typedef struct {
     int val;
 } libxl_defbool;
 
+#define LIBXL__DEFBOOL_DEFAULT (0)
+#define LIBXL__DEFBOOL_FALSE (-1)
+#define LIBXL__DEFBOOL_TRUE (1)
+
+#define DEFINE_BOOL(name, _val) \
+    libxl_defbool name = { .val = _val }
+#define DEFINE_FALSE_BOOL(name) DEFINE_BOOL(name, LIBXL__DEFBOOL_FALSE)
+#define DEFINE_TRUE_BOOL(name) DEFINE_BOOL(name, LIBXL__DEFBOOL_TRUE)
+
 void libxl_defbool_set(libxl_defbool *db, bool b);
 /* Resets to default */
 void libxl_defbool_unset(libxl_defbool *db);
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index 3c141bf..4b42b87 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -46,7 +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;
+DEFINE_FALSE_BOOL(claim_mode);
 
 static xentoollog_level minmsglevel = XTL_PROGRESS;
 
@@ -170,7 +170,6 @@ 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);

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

[Ian Campbell]

On Fri, 2013-04-12 at 21:07 +0100, Konrad Rzeszutek Wilk wrote:
> On Fri, Apr 12, 2013 at 03:51:04PM -0400, Konrad Rzeszutek Wilk wrote:
> > > Of these I prefer 1.  Opinions ?  Whatever we do needs to be in 4.3.
> > 
> > There is also option 5. Define a new macro:
> > 
> > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> > index 4922313..4a6ee76 100644
> > --- a/tools/libxl/libxl.h
> > +++ b/tools/libxl/libxl.h
> > @@ -359,6 +359,11 @@ typedef struct {
> >      int val;
> >  } libxl_defbool;
> >  
> > +#define DEFINE_BOOL(name, _val) \
> > +    libxl_defbool name = { .val = _val }
> > +#define DEFINE_FALSE_BOOL(name) DEFINE_BOOL(name, LIBXL__DEFBOOL_FALSE)
> > +#define DEFINE_TRUE_BOOL(name) DEFINE_BOOL(name, LIBXL__DEFBOOL_TRUE)
> > +
> >  void libxl_defbool_set(libxl_defbool *db, bool b);
> >  /* Resets to default */
> >  void libxl_defbool_unset(libxl_defbool *db);
> > 
> > 
> > And use DEFINE_FALSE_BOOL(claim_mode) in the xl.h file.
> > 
> 
> Correction (But now that I look at it, it is not that much nicer as you
> end up with LIBXL__DEFBOOL in the header file. <sigh>

If this is the right way to go (I'm not sure) then I thing you just want
to stop using defbool and use regular booleans, like the other xl
options do.

Ian.

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

[Ian Campbell]

On Fri, 2013-04-12 at 19:03 +0100, Ian Jackson wrote:
> Ian Jackson writes ("Re: [PATCH 2/6] xl: Implement XENMEM_claim_pages support via 'claim_mode' global config"):
> > Sorry, I just spotted this.  I think the libxl_defbool_setdefault
> > shouldn't be there.  The defbool should be initialised to "default",
> > which can be done by setting it to 0, as per:
> > 
> >   * To allow users of the library to naively select all defaults this
> >   * state is represented as 0. False is < 0 and True is > 0.
> > 
> > in libxl.h.  And since it's a variable of static duration the C
> > implementation will initialise it to 0.  So just deleting the
> > setdefault is right.
> > 
> > The result is that the default is set in libxl, only.
> 
> Konrad points out that without this, xl can't easily find out whether
> the claim mode is enabled or not.

Does it need to know? Is the presence of any non-zero value for a claim
enough indication for each function which might care to make a local
decision? At least nothing in this particular patch appears to care what
libxl's default is.

Is this setting supposed to be global (at either the host or specific
toolstack level) or is it supposed to be per-domain?

If it is at the toolstack level then shouldn't it be set in the
libxl_ctx instead of libxl_domain_build_info? If it is per-domain then
shouldn't there be the possibility of an override in the domain config?

If its supposed to be host wide then that seems to argue for a
requirement for a libxl specific configuration file, so that all
toolstacks (at least those which use libxl) can be configured. The xapi
guys were asking me about the possibility of such settings last week in
the context of host wide driver domain policy...

Anyway, back to the original point of this mail, assuming my questions
above haven't made that moot:

> Options are:
> 
> 1. Leave it as it is, default set in libxl but overridden by
>    separate setting in xl (perhaps we should add a comment).
>    Tolerable.
> 
> 2. Move the default setting out of libxl entirely, so callers
>    must pass 0 or 1.  (I don't approve of this; we might want
>    to change the behaviour of naive toolstacks in the future.)

Agree that this is best avoided.

> 3. Provide a new interface to libxl which allows the claim
>    default to be retrieved.  Palaver.

Could incorporate it into whichever of the libxl_*info interfaces seems
most appropriate. libxl_physinfo contains a lot of the associated free
memory values, so although claim mode isn't really "phys" perhaps that's
the best place for it?

> 4. Have xl operations which need to know the default claim mode
>    set up a dummy domain creation config, ask libxl to set the
>    defaults in it, and then read out the value.  Very ugly.

Very.

> Of these I prefer 1.  Opinions ?  Whatever we do needs to be in 4.3.

1 is probably the best of the bunch but I note that in that case the
implementation in xl should be just:

   xl.c:
	int claim_mode; /* = 0 */

   xl.c:parse_global_config():
	if (!xlu_cfg_get_long (config, "claim_mode", &l, 0))
            claim_mode = l;

   xl_cmdimpl.c:parse_config_data():
        libxl_defbool_set_default(&b_info->claim_mode, claim_mode)

i.e. xl's glboal claim mode setting is just a bool, not a defbool.

Ian.

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

[konrad wilk]

On 4/15/2013 5:34 AM, Ian Campbell wrote:
> On Fri, 2013-04-12 at 19:03 +0100, Ian Jackson wrote:
>> Ian Jackson writes ("Re: [PATCH 2/6] xl: Implement XENMEM_claim_pages support via 'claim_mode' global config"):
>>> Sorry, I just spotted this.  I think the libxl_defbool_setdefault
>>> shouldn't be there.  The defbool should be initialised to "default",
>>> which can be done by setting it to 0, as per:
>>>
>>>    * To allow users of the library to naively select all defaults this
>>>    * state is represented as 0. False is < 0 and True is > 0.
>>>
>>> in libxl.h.  And since it's a variable of static duration the C
>>> implementation will initialise it to 0.  So just deleting the
>>> setdefault is right.
>>>
>>> The result is that the default is set in libxl, only.
>> Konrad points out that without this, xl can't easily find out whether
>> the claim mode is enabled or not.
> Does it need to know? Is the presence of any non-zero value for a claim
> enough indication for each function which might care to make a local
> decision? At least nothing in this particular patch appears to care what
> libxl's default is.

The issue was that if you try to do libxl_get_defbool and the bool is a 
default - it will
blow up with an assert.
> Is this setting supposed to be global (at either the host or specific
> toolstack level) or is it supposed to be per-domain?
Global
>
> If it is at the toolstack level then shouldn't it be set in the
> libxl_ctx instead of libxl_domain_build_info? If it is per-domain then
> shouldn't there be the possibility of an override in the domain config?
>
> If its supposed to be host wide then that seems to argue for a
> requirement for a libxl specific configuration file, so that all
> toolstacks (at least those which use libxl) can be configured. The xapi
> guys were asking me about the possibility of such settings last week in
> the context of host wide driver domain policy...
>
> Anyway, back to the original point of this mail, assuming my questions
> above haven't made that moot:
>
>> Options are:
>>
>> 1. Leave it as it is, default set in libxl but overridden by
>>     separate setting in xl (perhaps we should add a comment).
>>     Tolerable.
>>
>> 2. Move the default setting out of libxl entirely, so callers
>>     must pass 0 or 1.  (I don't approve of this; we might want
>>     to change the behaviour of naive toolstacks in the future.)
> Agree that this is best avoided.
>
>> 3. Provide a new interface to libxl which allows the claim
>>     default to be retrieved.  Palaver.
> Could incorporate it into whichever of the libxl_*info interfaces seems
> most appropriate. libxl_physinfo contains a lot of the associated free
> memory values, so although claim mode isn't really "phys" perhaps that's
> the best place for it?
>
>> 4. Have xl operations which need to know the default claim mode
>>     set up a dummy domain creation config, ask libxl to set the
>>     defaults in it, and then read out the value.  Very ugly.
> Very.
>
>> Of these I prefer 1.  Opinions ?  Whatever we do needs to be in 4.3.
> 1 is probably the best of the bunch but I note that in that case the
> implementation in xl should be just:
>
>     xl.c:
> 	int claim_mode; /* = 0 */
>
>     xl.c:parse_global_config():
> 	if (!xlu_cfg_get_long (config, "claim_mode", &l, 0))
>              claim_mode = l;
>
>     xl_cmdimpl.c:parse_config_data():
>          libxl_defbool_set_default(&b_info->claim_mode, claim_mode)
>
> i.e. xl's glboal claim mode setting is just a bool, not a defbool.

Yes. That will work too. This was how the earlier versions had it.

I will post a new version when I back in office tomorrow.

Thanks!
>
> Ian.
>

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

[Ian Campbell]

On Tue, 2013-04-16 at 00:20 +0100, konrad wilk wrote:
> On 4/15/2013 5:34 AM, Ian Campbell wrote:
> > On Fri, 2013-04-12 at 19:03 +0100, Ian Jackson wrote:
> >> Ian Jackson writes ("Re: [PATCH 2/6] xl: Implement XENMEM_claim_pages support via 'claim_mode' global config"):
> >>> Sorry, I just spotted this.  I think the libxl_defbool_setdefault
> >>> shouldn't be there.  The defbool should be initialised to "default",
> >>> which can be done by setting it to 0, as per:
> >>>
> >>>    * To allow users of the library to naively select all defaults this
> >>>    * state is represented as 0. False is < 0 and True is > 0.
> >>>
> >>> in libxl.h.  And since it's a variable of static duration the C
> >>> implementation will initialise it to 0.  So just deleting the
> >>> setdefault is right.
> >>>
> >>> The result is that the default is set in libxl, only.
> >> Konrad points out that without this, xl can't easily find out whether
> >> the claim mode is enabled or not.
> > Does it need to know? Is the presence of any non-zero value for a claim
> > enough indication for each function which might care to make a local
> > decision? At least nothing in this particular patch appears to care what
> > libxl's default is.
> 
> The issue was that if you try to do libxl_get_defbool and the bool is a 
> default - it will
> blow up with an assert.

My real question is who (outside of libxl) is doing that
libxl_get_defbool and why?

> > Is this setting supposed to be global (at either the host or specific
> > toolstack level) or is it supposed to be per-domain?
> Global

Hrm, this suggests that the approach here (which is inherently
per-domain) is wrong then? i.e. this part of my original mail applies:
> > If its supposed to be host wide then that seems to argue for a
> > requirement for a libxl specific configuration file, so that all
> > toolstacks (at least those which use libxl) can be configured. The xapi
> > guys were asking me about the possibility of such settings last week in
> > the context of host wide driver domain policy...

> > Anyway, back to the original point of this mail, assuming my questions
> > above haven't made that moot:

I think it has :-(

[...]
> >     xl.c:
> > 	int claim_mode; /* = 0 */
> >
> >     xl.c:parse_global_config():
> > 	if (!xlu_cfg_get_long (config, "claim_mode", &l, 0))
> >              claim_mode = l;
> >
> >     xl_cmdimpl.c:parse_config_data():
> >          libxl_defbool_set_default(&b_info->claim_mode, claim_mode)
> >
> > i.e. xl's glboal claim mode setting is just a bool, not a defbool.
> 
> Yes. That will work too. This was how the earlier versions had it.

Unfortunately this approach is only really valid if claim is per-domain,
if it is per host (i.e. global) as you suggest then the approach needs
to be entirely different AFAICT.

Ian.

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

[George Dunlap]

On 12/04/13 18:20, Ian Jackson wrote:
> Konrad Rzeszutek Wilk writes ("Re: [PATCH v15] claim and its friends for allocating multiple self-ballooning guests."):
>> I am just waiting for either Ian's to Ack it and stick it in the tree.
> This series is very close.  I'm picking tiny nits.  In the event it
> doesn't make feature code freeze I'd like to propose a freeze
> exception for it.  The hypervisor side is in tree already and the code
> has been floating around on the list for a long time.  The tools parts
> are pretty obvious and getting the API design right is what has been
> taking the extra time.

The key question here from the "code freeze" perspective is, "Will 
accepting this patch now potentially cause a regression which will delay 
the release?"

  -George

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

[Ian Jackson]

George Dunlap writes ("Re: [PATCH v15] claim and its friends for allocating multiple self-ballooning guests."):
> On 12/04/13 18:20, Ian Jackson wrote:
> > This series is very close.  I'm picking tiny nits.  In the event it
> > doesn't make feature code freeze I'd like to propose a freeze
> > exception for it.  The hypervisor side is in tree already and the code
> > has been floating around on the list for a long time.  The tools parts
> > are pretty obvious and getting the API design right is what has been
> > taking the extra time.
> 
> The key question here from the "code freeze" perspective is, "Will 
> accepting this patch now potentially cause a regression which will delay 
> the release?"

No.

Ian.

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

[George Dunlap]

On 16/04/13 11:57, Ian Jackson wrote:
> George Dunlap writes ("Re: [PATCH v15] claim and its friends for allocating multiple self-ballooning guests."):
>> On 12/04/13 18:20, Ian Jackson wrote:
>>> This series is very close.  I'm picking tiny nits.  In the event it
>>> doesn't make feature code freeze I'd like to propose a freeze
>>> exception for it.  The hypervisor side is in tree already and the code
>>> has been floating around on the list for a long time.  The tools parts
>>> are pretty obvious and getting the API design right is what has been
>>> taking the extra time.
>> The key question here from the "code freeze" perspective is, "Will
>> accepting this patch now potentially cause a regression which will delay
>> the release?"
> No.

Then it looks good to me.  From a release perspective:

Acked-by: George Dunlap <george.dunlap@eu.citrix.com>