Please review the patches attached. All of them have been Acked/Reviewed-by
and reworked per the last posting.
If they are OK please pull the following branch:
git://xenbits.xen.org/people/konradwilk/xen.git stable/for-xen-4.3.v2
Konrad Rzeszutek Wilk (5):
MAINTAINERS: Change tmem maintainer.
hypervisor/xen/tools: Remove the XENMEM_get_oustanding_pages and provide
the data via xc_phys_info
libxl: Change claim_mode from bool to int.
libxl: claim: Print the values in ''xl info''
unconditionally
libxl: Make ''xl vcpu-set'' work properly on overcommited
hosts with an override.
MAINTAINERS | 2 +-
tools/libxc/xc_domain.c | 9 ------
tools/libxc/xenctrl.h | 2 --
tools/libxl/libxl.c | 15 +---------
tools/libxl/libxl.h | 1 -
tools/libxl/libxl_types.idl | 1 +
tools/libxl/xl.c | 6 ++--
tools/libxl/xl.h | 2 +-
tools/libxl/xl_cmdimpl.c | 58 +++++++++++++++++++++----------------
tools/libxl/xl_cmdtable.c | 3 +-
xen/common/memory.c | 8 -----
xen/common/page_alloc.c | 7 +++--
xen/common/sysctl.c | 3 +-
xen/include/public/memory.h | 7 -----
xen/include/public/sysctl.h | 3 +-
xen/include/xen/mm.h | 2 +-
xen/include/xsm/dummy.h | 6 ----
xen/include/xsm/xsm.h | 6 ----
xen/xsm/dummy.c | 1 -
xen/xsm/flask/hooks.c | 7 -----
xen/xsm/flask/policy/access_vectors | 2 +-
21 files changed, 53 insertions(+), 98 deletions(-)
Konrad Rzeszutek Wilk
2013-May-13 19:29 UTC
[PATCH 1/5] MAINTAINERS: Change tmem maintainer.
Konrad has graduated to becoming an maintainer in the Xen hypervisor. Acked-by: Ian Campbell <ian.campbell@citrix.com> Acked-by: Dan Magenheimer <dan.magenheimer@oracle.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 7c672c8..b1e8d23 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -262,7 +262,7 @@ S: Supported F: tools/ TRANSCENDENT MEMORY (TMEM) -M: Dan Magenheimer <dan.magenheimer@oracle.com> +M: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> W: http://oss.oracle.com/projects/tmem S: Supported F: xen/common/tmem* -- 1.8.1.4
Konrad Rzeszutek Wilk
2013-May-13 19:29 UTC
[PATCH 2/5] hypervisor/xen/tools: Remove the XENMEM_get_oustanding_pages and provide the data via xc_phys_info
During the review of the patches it was noticed that there exists
a race wherein the ''free_memory'' value consists of information
from
two hypercalls. That is the XEN_SYSCTL_physinfo and
XENMEM_get_outstanding_pages.
The free memory the host has available for guest is the difference between
the ''free_pages'' (from XEN_SYSCTL_physinfo) and
''outstanding_pages''. As they
are two hypercalls many things can happen in between the execution of them.
This patch resolves this by eliminating the XENMEM_get_outstanding_pages
hypercall and providing the free_pages and outstanding_pages information
via the xc_phys_info structure.
It also removes the XSM hooks and adds locking as needed.
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Reviewed-by: Tim Deegan <tim@xen.org>
Acked-by: Keir Fraser <keir.xen@gmail.com>
[v1: Fix missing XSM hooks cleanups, fixed get_outstanding_claims fnc,
add Acked/Reviewed-by]
[v2: s/%ld/PRIu64/ as we don''t use ''long'' anymore]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
tools/libxc/xc_domain.c | 9 ---------
tools/libxc/xenctrl.h | 2 --
tools/libxl/libxl.c | 15 +--------------
tools/libxl/libxl.h | 1 -
tools/libxl/libxl_types.idl | 1 +
tools/libxl/xl_cmdimpl.c | 16 +++-------------
xen/common/memory.c | 8 --------
xen/common/page_alloc.c | 7 +++++--
xen/common/sysctl.c | 3 ++-
xen/include/public/memory.h | 7 -------
xen/include/public/sysctl.h | 3 ++-
xen/include/xen/mm.h | 2 +-
xen/include/xsm/dummy.h | 6 ------
xen/include/xsm/xsm.h | 6 ------
xen/xsm/dummy.c | 1 -
xen/xsm/flask/hooks.c | 7 -------
xen/xsm/flask/policy/access_vectors | 2 +-
17 files changed, 16 insertions(+), 80 deletions(-)
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index bb71cca..3257e2a 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -873,15 +873,6 @@ int xc_domain_claim_pages(xc_interface *xch,
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,
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index c024af4..40ee8fc 100644
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -1182,8 +1182,6 @@ 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/libxl/libxl.c b/tools/libxl/libxl.c
index 87bda72..dfba755 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3941,6 +3941,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo
*physinfo)
physinfo->total_pages = xcphysinfo.total_pages;
physinfo->free_pages = xcphysinfo.free_pages;
physinfo->scrub_pages = xcphysinfo.scrub_pages;
+ physinfo->outstanding_pages = xcphysinfo.outstanding_pages;
l = xc_sharing_freed_pages(ctx->xch);
if (l == -ENOSYS) {
l = 0;
@@ -4104,20 +4105,6 @@ 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 pages */
- return l;
-}
-
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 ef96bce..0bc005e 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -624,7 +624,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);
-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/libxl_types.idl b/tools/libxl/libxl_types.idl
index ecf1f0b..8262cba 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -487,6 +487,7 @@ libxl_physinfo = Struct("physinfo", [
("total_pages", uint64),
("free_pages", uint64),
("scrub_pages", uint64),
+ ("outstanding_pages", uint64),
("sharing_freed_pages", uint64),
("sharing_used_frames", uint64),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index c1a969b..edf0325 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4576,21 +4576,11 @@ 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);
@@ -4610,15 +4600,15 @@ 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 - claims) / i);
+ printf("free_memory : %"PRIu64"\n",
(info.free_pages - info.outstanding_pages) / 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_defbool_val(claim_mode) || info.outstanding_pages)
+ printf("outstanding_claims : %"PRIu64"\n",
info.outstanding_pages / i);
if (!libxl_get_freecpus(ctx, &cpumap)) {
libxl_for_each_bit(i, cpumap)
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 3239d53..06a0d0a 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -737,14 +737,6 @@ long do_memory_op(unsigned long cmd,
XEN_GUEST_HANDLE_PARAM(void) arg)
break;
- case XENMEM_get_outstanding_pages:
- rc = xsm_xenmem_get_outstanding_pages(XSM_PRIV);
-
- if ( !rc )
- rc = get_outstanding_claims();
-
- break;
-
default:
rc = arch_memory_op(op, arg);
break;
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 203f77a..2162ef1 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -380,9 +380,12 @@ out:
return ret;
}
-long get_outstanding_claims(void)
+void get_outstanding_claims(uint64_t *free_pages, uint64_t *outstanding_pages)
{
- return outstanding_claims;
+ spin_lock(&heap_lock);
+ *outstanding_pages = outstanding_claims;
+ *free_pages = avail_domheap_pages();
+ spin_unlock(&heap_lock);
}
static unsigned long init_node_heap(int node, unsigned long mfn,
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 31f9650..117e095 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -264,7 +264,8 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t)
u_sysctl)
pi->max_node_id = MAX_NUMNODES-1;
pi->max_cpu_id = nr_cpu_ids - 1;
pi->total_pages = total_pages;
- pi->free_pages = avail_domheap_pages();
+ /* Protected by lock */
+ get_outstanding_claims(&pi->free_pages,
&pi->outstanding_pages);
pi->scrub_pages = 0;
pi->cpu_khz = cpu_khz;
arch_do_physinfo(pi);
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 51d5254..7a26dee 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -459,13 +459,6 @@ DEFINE_XEN_GUEST_HANDLE(xen_mem_sharing_op_t);
* The zero value is appropiate.
*/
-/*
- * Get the number of pages currently claimed (but not yet
"possessed")
- * across all domains. The caller must be privileged but otherwise
- * the call never fails.
- */
-#define XENMEM_get_outstanding_pages 25
-
#endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */
#endif /* __XEN_PUBLIC_MEMORY_H__ */
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 03710d8..8437d31 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -34,7 +34,7 @@
#include "xen.h"
#include "domctl.h"
-#define XEN_SYSCTL_INTERFACE_VERSION 0x00000009
+#define XEN_SYSCTL_INTERFACE_VERSION 0x0000000A
/*
* Read console content from Xen buffer ring.
@@ -101,6 +101,7 @@ struct xen_sysctl_physinfo {
uint64_aligned_t total_pages;
uint64_aligned_t free_pages;
uint64_aligned_t scrub_pages;
+ uint64_aligned_t outstanding_pages;
uint32_t hw_cap[8];
/* XEN_SYSCTL_PHYSCAP_??? */
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 28512fb..ac6e4eb 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -52,7 +52,7 @@ void free_xenheap_pages(void *v, unsigned int order);
/* Claim handling */
unsigned long domain_adjust_tot_pages(struct domain *d, long pages);
int domain_set_outstanding_pages(struct domain *d, unsigned long pages);
-long get_outstanding_claims(void);
+void get_outstanding_claims(uint64_t *free_pages, uint64_t *outstanding_pages);
/* Domain suballocator. These functions are *not* interrupt-safe.*/
void init_domheap_pages(paddr_t ps, paddr_t pe);
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index a872056..cc0a5a8 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -259,12 +259,6 @@ static XSM_INLINE int xsm_claim_pages(XSM_DEFAULT_ARG
struct domain *d)
return xsm_default_action(action, current->domain, d);
}
-static XSM_INLINE int xsm_xenmem_get_outstanding_pages(XSM_DEFAULT_VOID)
-{
- XSM_ASSERT_ACTION(XSM_PRIV);
- return xsm_default_action(action, current->domain, NULL);
-}
-
static XSM_INLINE int xsm_evtchn_unbound(XSM_DEFAULT_ARG struct domain *d,
struct evtchn *chn,
domid_t id2)
{
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 58a4fbb..1939453 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -93,7 +93,6 @@ struct xsm_operations {
int (*add_to_physmap) (struct domain *d1, struct domain *d2);
int (*remove_from_physmap) (struct domain *d1, struct domain *d2);
int (*claim_pages) (struct domain *d);
- int (*xenmem_get_outstanding_pages) (void);
int (*console_io) (struct domain *d, int cmd);
@@ -360,11 +359,6 @@ static inline int xsm_claim_pages(xsm_default_t def, struct
domain *d)
return xsm_ops->claim_pages(d);
}
-static inline int xsm_xenmem_get_outstanding_pages(xsm_default_t def)
-{
- return xsm_ops->xenmem_get_outstanding_pages();
-}
-
static inline int xsm_console_io (xsm_default_t def, struct domain *d, int cmd)
{
return xsm_ops->console_io(d, cmd);
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index 937761f..31e4f73 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -67,7 +67,6 @@ void xsm_fixup_ops (struct xsm_operations *ops)
set_to_dummy_if_null(ops, memory_stat_reservation);
set_to_dummy_if_null(ops, memory_pin_page);
set_to_dummy_if_null(ops, claim_pages);
- set_to_dummy_if_null(ops, xenmem_get_outstanding_pages);
set_to_dummy_if_null(ops, console_io);
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index bb10de3..fa0589a 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -422,12 +422,6 @@ static int flask_claim_pages(struct domain *d)
return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__SETCLAIM);
}
-static int flask_xenmem_get_outstanding_pages(void)
-{
- return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN,
- XEN__HEAP, NULL);
-}
-
static int flask_console_io(struct domain *d, int cmd)
{
u32 perm;
@@ -1504,7 +1498,6 @@ static struct xsm_operations flask_ops = {
.memory_stat_reservation = flask_memory_stat_reservation,
.memory_pin_page = flask_memory_pin_page,
.claim_pages = flask_claim_pages,
- .xenmem_get_outstanding_pages = flask_xenmem_get_outstanding_pages,
.console_io = flask_console_io,
diff --git a/xen/xsm/flask/policy/access_vectors
b/xen/xsm/flask/policy/access_vectors
index 544c3ba..5dfe13b 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -54,7 +54,7 @@ class xen
debug
# XEN_SYSCTL_getcpuinfo, XENPF_get_cpu_version, XENPF_get_cpuinfo
getcpuinfo
-# XEN_SYSCTL_availheap, XENMEM_get_outstanding_pages
+# XEN_SYSCTL_availheap
heap
# XEN_SYSCTL_get_pmstat, XEN_SYSCTL_pm_op, XENPF_set_processor_pminfo,
# XENPF_core_parking
--
1.8.1.4
Konrad Rzeszutek Wilk
2013-May-13 19:29 UTC
[PATCH 3/5] libxl: Change claim_mode from bool to int.
During the review it was noticed that it would be better if internally
the claim_mode was held as an ''int'' instead of a
''bool''. The reason
is that during the startup of xl, one has call the libxl_defbool_setdefault.
otherwise any usage of claim_mode would result in assert break.
The assert is due to the fact that using defbool without any set
values (either true of false) will cause it hit an assertion.
If we use an ''int'' we don''t have to worry about it
and by default
the value of zero will suffice for checks whether the claim is
enabled or disabled.
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
[v1: Make by default b_info->claim_mode false to allow callers of
libxl to not care about claim_mode]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
tools/libxl/xl.c | 6 +++---
tools/libxl/xl.h | 2 +-
tools/libxl/xl_cmdimpl.c | 6 +++---
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index 3c141bf..1ce820c 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;
+int claim_mode = 0;
static xentoollog_level minmsglevel = XTL_PROGRESS;
@@ -170,8 +170,8 @@ 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);
+ if (!xlu_cfg_get_long (config, "claim_mode", &l, 0))
+ claim_mode = l;
xlu_cfg_destroy(config);
}
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 771b4af..5ad3e17 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -146,7 +146,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 int 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 edf0325..609ce49 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -734,7 +734,7 @@ 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;
+ libxl_defbool_set(&b_info->claim_mode, claim_mode);
if (xlu_cfg_get_string (config, "on_poweroff", &buf, 0))
buf = "destroy";
@@ -4607,7 +4607,7 @@ static void output_physinfo(void)
/*
* Only if enabled (claim_mode=1) or there are outstanding claims.
*/
- if (libxl_defbool_val(claim_mode) || info.outstanding_pages)
+ if (claim_mode || info.outstanding_pages)
printf("outstanding_claims : %"PRIu64"\n",
info.outstanding_pages / i);
if (!libxl_get_freecpus(ctx, &cpumap)) {
@@ -5911,7 +5911,7 @@ int main_claims(int argc, char **argv)
/* No options */
}
- if (!libxl_defbool_val(claim_mode))
+ if (!claim_mode)
fprintf(stderr, "claim_mode not enabled (see man
xl.conf).\n");
info = libxl_list_domain(ctx, &nb_domain);
--
1.8.1.4
Konrad Rzeszutek Wilk
2013-May-13 19:29 UTC
[PATCH 4/5] libxl: claim: Print the values in ''xl info'' unconditionally
During the review of "libxl: Change claim_mode from bool to int."
Ian Campbell suggested that the xl info should print the
claim information irregardless of the global claim_mode value.
Suggested-by: Ian Campbell <Ian.Campbell@citrix.com>
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
[v1: Fold the printf inside the loop]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
tools/libxl/xl_cmdimpl.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 609ce49..497d84d 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4603,13 +4603,8 @@ static void output_physinfo(void)
printf("free_memory : %"PRIu64"\n",
(info.free_pages - info.outstanding_pages) / 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 (claim_mode || info.outstanding_pages)
printf("outstanding_claims : %"PRIu64"\n",
info.outstanding_pages / i);
-
+ }
if (!libxl_get_freecpus(ctx, &cpumap)) {
libxl_for_each_bit(i, cpumap)
if (libxl_bitmap_test(&cpumap, i))
--
1.8.1.4
Konrad Rzeszutek Wilk
2013-May-13 19:29 UTC
[PATCH 5/5] libxl: Make ''xl vcpu-set'' work properly on overcommited hosts with an override.
The libxl_cpu_bitmap_alloc(..) function, if provided with a zero
value for max CPUs will call xc_get_max_cpus() which will retrieve
the number of physical CPUs the host has. This is usually
OK if the guest''s maxvcpus <= host pcpus. But if the value
is different, then the bitmap for VCPUs is limited by the
number of CPUs the host has.
This is incorrect as what we want is to hotplug in the guest
the amount of CPUs that the user specified on the command line
and not be limited by the amount of physical CPUs.
This means that a guest config like this:
vcpus=8
maxvcpus=32
and on a 4 PCPU machine doing
xl vcpu-set <guest name> 16
won''t work. This is b/c the the size of the bitmap is one byte
so it can only hold up to 8 VCPUs. Hence anything above that
is going to be ignored.
Note that this patch also fixes the bitmap setting - as it
would set all of the bits allowed. Meaning if the user had a 4PCPU
host we would still allow the user to set 8VCPUs. This second
iteration of the patch fixes this.
Note that all of the libxl_cpu_bitmap_[test|set] silently ignore
any test or sets above its size:
if (bit >= bitmap->size * 8)
return 0;
so we were never notified off this bug.
This patch warns the user if they are trying to do this. If the
user really wants to do this they have to provide the --ignore-host
parameter to bypass this check.
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
[v1: Add --ignore-host and also fix the bitmap being updated to
the full word instead of to the count of physical CPUs]
[v2: Fix spelling mistakes]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
tools/libxl/xl_cmdimpl.c | 35 +++++++++++++++++++++++++++++------
tools/libxl/xl_cmdtable.c | 3 ++-
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 497d84d..e13a64e 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4487,7 +4487,7 @@ int main_vcpupin(int argc, char **argv)
return 0;
}
-static void vcpuset(uint32_t domid, const char* nr_vcpus)
+static void vcpuset(uint32_t domid, const char* nr_vcpus, int check_host)
{
char *endptr;
unsigned int max_vcpus, i;
@@ -4499,7 +4499,22 @@ static void vcpuset(uint32_t domid, const char* nr_vcpus)
return;
}
- if (libxl_cpu_bitmap_alloc(ctx, &cpumap, 0)) {
+ /*
+ * Maximum amount of vCPUS the guest is allowed to set is limited
+ * by the host''s amount of pCPUs.
+ */
+ if (check_host) {
+ unsigned int host_cpu = libxl_get_max_cpus(ctx);
+ if (max_vcpus > host_cpu) {
+ fprintf(stderr, "You are overcommmitting! You have %d physical
" \
+ " CPUs and want %d vCPUs! Aborting, use --ignore-host
to " \
+ " continue\n", host_cpu, max_vcpus);
+ return;
+ }
+ /* NB: This also limits how many are set in the bitmap */
+ max_vcpus = (max_vcpus > host_cpu ? host_cpu : max_vcpus);
+ }
+ if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) {
fprintf(stderr, "libxl_cpu_bitmap_alloc failed\n");
return;
}
@@ -4514,13 +4529,21 @@ static void vcpuset(uint32_t domid, const char*
nr_vcpus)
int main_vcpuset(int argc, char **argv)
{
- int opt;
+ static struct option opts[] = {
+ {"ignore-host", 0, 0, ''i''},
+ {0, 0, 0, 0}
+ };
+ int opt, check_host = 1;
- SWITCH_FOREACH_OPT(opt, "", NULL, "vcpu-set", 2) {
- /* No options */
+ SWITCH_FOREACH_OPT(opt, "i", opts, "vcpu-set", 2) {
+ case ''i'':
+ check_host = 0;
+ break;
+ default:
+ break;
}
- vcpuset(find_domain(argv[optind]), argv[optind+1]);
+ vcpuset(find_domain(argv[optind]), argv[optind + 1], check_host);
return 0;
}
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 347302c..c98d9bb 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -217,7 +217,8 @@ struct cmd_spec cmd_table[] = {
{ "vcpu-set",
&main_vcpuset, 0, 1,
"Set the number of active VCPUs allowed for the domain",
- "<Domain> <vCPUs>",
+ "[option] <Domain> <vCPUs>",
+ "-i, --ignore-host Don''t limit the vCPU based on the host
CPU count",
},
{ "vm-list",
&main_vm_list, 0, 0,
--
1.8.1.4
>>> On 13.05.13 at 21:29, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > Konrad has graduated to becoming an maintainer in the Xen hypervisor.Why do you keep reposting this? It hardly can be against the tip of the staging tree anymore, as it got applied (with some hassle, as the patch had and continues to have a stray blank at the context line "W: ...") 3 days ago. Jan> Acked-by: Ian Campbell <ian.campbell@citrix.com> > Acked-by: Dan Magenheimer <dan.magenheimer@oracle.com> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > --- > MAINTAINERS | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7c672c8..b1e8d23 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -262,7 +262,7 @@ S: Supported > F: tools/ > > TRANSCENDENT MEMORY (TMEM) > -M: Dan Magenheimer <dan.magenheimer@oracle.com> > +M: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > W: http://oss.oracle.com/projects/tmem > S: Supported > F: xen/common/tmem* > -- > 1.8.1.4 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Ian Campbell
2013-May-14 09:36 UTC
Re: [PATCH 2/5] hypervisor/xen/tools: Remove the XENMEM_get_oustanding_pages and provide the data via xc_phys_info
On Mon, 2013-05-13 at 20:29 +0100, Konrad Rzeszutek Wilk wrote:> During the review of the patches it was noticed that there exists > a race wherein the ''free_memory'' value consists of information from > two hypercalls. That is the XEN_SYSCTL_physinfo and XENMEM_get_outstanding_pages. > > The free memory the host has available for guest is the difference between > the ''free_pages'' (from XEN_SYSCTL_physinfo) and ''outstanding_pages''. As they > are two hypercalls many things can happen in between the execution of them. > > This patch resolves this by eliminating the XENMEM_get_outstanding_pages > hypercall and providing the free_pages and outstanding_pages information > via the xc_phys_info structure. > > It also removes the XSM hooks and adds locking as needed. > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > Reviewed-by: Tim Deegan <tim@xen.org> > Acked-by: Keir Fraser <keir.xen@gmail.com> > [v1: Fix missing XSM hooks cleanups, fixed get_outstanding_claims fnc, > add Acked/Reviewed-by] > [v2: s/%ld/PRIu64/ as we don''t use ''long'' anymore] > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>I think we''ve discussed this before but your tags should be in a roughly chronological order, not backwards in time as you have them. Or at least that''s how everyone else does it. Also the intra-version changelog should go afterwards following a "---" so it is stripped from the final commit. I''ve fixed this as I applied #2..#5. #1 was already in since before your previous posting. Ian.
Konrad Rzeszutek Wilk
2013-May-14 13:26 UTC
Re: [PATCH 1/5] MAINTAINERS: Change tmem maintainer.
On Tue, May 14, 2013 at 08:47:18AM +0100, Jan Beulich wrote:> >>> On 13.05.13 at 21:29, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > > Konrad has graduated to becoming an maintainer in the Xen hypervisor. > > Why do you keep reposting this? It hardly can be against the tip > of the staging tree anymore, as it got applied (with some hassle, > as the patch had and continues to have a stray blank at the > context line "W: ...") 3 days ago.I am not sure why there is a stray blank. The patch certainly did not touch the W line. The reason for the repost is me being unclear how the process works now that we are using git. I am used to sending ''git pull'' (in which case it does not matter whether the patch had been applied already or is still in my tree), but the mercurial process favored applying each patch by hand. Hence I sent this out as a hybrid - patches for those that prefer applying it by hand and ''git pull'' in case a whole-sale ''git pull'' is done. But it is unclear to me whether anybody except Ian Jackson is doing the git pull and who is actually responsible for putting the patches in - especially as some of these cross multiple boundaries between maintainers. And whether some maintainers prefer git pulls or just plain old regular patches. From what I have gleamed so far, it is: Ian J - git pull Ian C, Jan - plain old patches.> > Jan > > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > > Acked-by: Dan Magenheimer <dan.magenheimer@oracle.com> > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > --- > > MAINTAINERS | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 7c672c8..b1e8d23 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -262,7 +262,7 @@ S: Supported > > F: tools/ > > > > TRANSCENDENT MEMORY (TMEM) > > -M: Dan Magenheimer <dan.magenheimer@oracle.com> > > +M: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > W: http://oss.oracle.com/projects/tmem > > S: Supported > > F: xen/common/tmem* > > -- > > 1.8.1.4 > > > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xen.org > > http://lists.xen.org/xen-devel > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >
Konrad Rzeszutek Wilk
2013-May-14 13:28 UTC
Re: [PATCH 2/5] hypervisor/xen/tools: Remove the XENMEM_get_oustanding_pages and provide the data via xc_phys_info
On Tue, May 14, 2013 at 10:36:53AM +0100, Ian Campbell wrote:> On Mon, 2013-05-13 at 20:29 +0100, Konrad Rzeszutek Wilk wrote: > > During the review of the patches it was noticed that there exists > > a race wherein the ''free_memory'' value consists of information from > > two hypercalls. That is the XEN_SYSCTL_physinfo and XENMEM_get_outstanding_pages. > > > > The free memory the host has available for guest is the difference between > > the ''free_pages'' (from XEN_SYSCTL_physinfo) and ''outstanding_pages''. As they > > are two hypercalls many things can happen in between the execution of them. > > > > This patch resolves this by eliminating the XENMEM_get_outstanding_pages > > hypercall and providing the free_pages and outstanding_pages information > > via the xc_phys_info structure. > > > > It also removes the XSM hooks and adds locking as needed. > > > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > > Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > Reviewed-by: Tim Deegan <tim@xen.org> > > Acked-by: Keir Fraser <keir.xen@gmail.com> > > [v1: Fix missing XSM hooks cleanups, fixed get_outstanding_claims fnc, > > add Acked/Reviewed-by] > > [v2: s/%ld/PRIu64/ as we don''t use ''long'' anymore] > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > I think we''ve discussed this before but your tags should be in a roughly > chronological order, not backwards in time as you have them. Or at least > that''s how everyone else does it.Grrr.. Old habbits. Will make sure to have it in the prefer order.> > Also the intra-version changelog should go afterwards following a "---" > so it is stripped from the final commit.OK, will do that in the future as well for Xen patches.> > I''ve fixed this as I applied #2..#5. #1 was already in since before your > previous posting.Thank you.> > Ian. > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >
On Tue, 2013-05-14 at 14:26 +0100, Konrad Rzeszutek Wilk wrote:> And whether some maintainers prefer git pulls or just plain old regular patches.AFAIK the default remains patches for all committers. Remember that there are multiple people who might commit any given series once it has ACKs from the appropriate maintainer(s). In theory any committer can apply any patch with acks from the right maintainers, in practice we tend to let the committers who are also maintainers of an area take care of it. If a series crosses boundaries we use our judgement or discuss it. Sometimes a maintainer might ask for a git pull once a series has been through several iterations as a convenience, e.g. if it is big or complex etc. I''ve done that occasionally and I know Ian has too. But this is the exception not the rule. That''s not to say a git request as well as the default patchbomb isn''t potentially useful, but resending already applied patches should be avoided. Since we generally deal in patches it is normal for resends to be rebased onto the latest tip anyway, which is what the effect of applying the patches will be. Ian.