Ian Campbell
2012-Sep-03 12:48 UTC
[PATCH] libxl: handle errors from xc_sharing_* info functions
# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1346676461 -3600
# Node ID 99b7a5af9b3059bac2732a601fb1d574b9f02975
# Parent c4e822e1b491bb7efa962b38fff6f007f01596b5
libxl: handle errors from xc_sharing_* info functions
On a 32 bit hypervisor xl info currently reports:
sharing_freed_memory : 72057594037927935
sharing_used_memory : 72057594037927935
Eat the ENOSYS and turn it into 0. Log and propagate other errors.
I don''t have a 32 bit system handy, so tested on x86_64 with a libxc
hacked to return -ENOSYS and -EINVAL.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
diff -r c4e822e1b491 -r 99b7a5af9b30 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c Mon Sep 03 09:59:35 2012 +0100
+++ b/tools/libxl/libxl.c Mon Sep 03 13:47:41 2012 +0100
@@ -3622,6 +3622,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, l
{
xc_physinfo_t xcphysinfo = { 0 };
int rc;
+ long l;
rc = xc_physinfo(ctx->xch, &xcphysinfo);
if (rc != 0) {
@@ -3636,8 +3637,20 @@ int libxl_get_physinfo(libxl_ctx *ctx, l
physinfo->total_pages = xcphysinfo.total_pages;
physinfo->free_pages = xcphysinfo.free_pages;
physinfo->scrub_pages = xcphysinfo.scrub_pages;
- physinfo->sharing_freed_pages = xc_sharing_freed_pages(ctx->xch);
- physinfo->sharing_used_frames = xc_sharing_used_frames(ctx->xch);
+ l = xc_sharing_freed_pages(ctx->xch);
+ if ( l < 0 && l != -ENOSYS ) {
+ LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, l,
+ "getting sharing freed pages");
+ return ERROR_FAIL;
+ }
+ physinfo->sharing_freed_pages = (l == -ENOSYS) ? 0 : l;
+ l = xc_sharing_used_frames(ctx->xch);
+ if ( l < 0 && l != -ENOSYS ) {
+ LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, l,
+ "getting sharing used frames");
+ return ERROR_FAIL;
+ }
+ physinfo->sharing_used_frames = (l == -ENOSYS) ? 0 : l;
physinfo->nr_nodes = xcphysinfo.nr_nodes;
memcpy(physinfo->hw_cap,xcphysinfo.hw_cap, sizeof(physinfo->hw_cap));
Andres Lagar-Cavilla
2012-Sep-03 17:52 UTC
Re: [PATCH] libxl: handle errors from xc_sharing_* info functions
On Sep 3, 2012, at 8:48 AM, Ian Campbell wrote:> # HG changeset patch > # User Ian Campbell <ian.campbell@citrix.com> > # Date 1346676461 -3600 > # Node ID 99b7a5af9b3059bac2732a601fb1d574b9f02975 > # Parent c4e822e1b491bb7efa962b38fff6f007f01596b5 > libxl: handle errors from xc_sharing_* info functions > > On a 32 bit hypervisor xl info currently reports: > sharing_freed_memory : 72057594037927935 > sharing_used_memory : 72057594037927935 > > Eat the ENOSYS and turn it into 0. Log and propagate other errors. > > I don''t have a 32 bit system handy, so tested on x86_64 with a libxc > hacked to return -ENOSYS and -EINVAL. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>Acked-by: Andres Lagar-Cavilla <andres@lagarcavilla.org> Good catch! Andres> > diff -r c4e822e1b491 -r 99b7a5af9b30 tools/libxl/libxl.c > --- a/tools/libxl/libxl.c Mon Sep 03 09:59:35 2012 +0100 > +++ b/tools/libxl/libxl.c Mon Sep 03 13:47:41 2012 +0100 > @@ -3622,6 +3622,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, l > { > xc_physinfo_t xcphysinfo = { 0 }; > int rc; > + long l; > > rc = xc_physinfo(ctx->xch, &xcphysinfo); > if (rc != 0) { > @@ -3636,8 +3637,20 @@ int libxl_get_physinfo(libxl_ctx *ctx, l > physinfo->total_pages = xcphysinfo.total_pages; > physinfo->free_pages = xcphysinfo.free_pages; > physinfo->scrub_pages = xcphysinfo.scrub_pages; > - physinfo->sharing_freed_pages = xc_sharing_freed_pages(ctx->xch); > - physinfo->sharing_used_frames = xc_sharing_used_frames(ctx->xch); > + l = xc_sharing_freed_pages(ctx->xch); > + if ( l < 0 && l != -ENOSYS ) { > + LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, l, > + "getting sharing freed pages"); > + return ERROR_FAIL; > + } > + physinfo->sharing_freed_pages = (l == -ENOSYS) ? 0 : l; > + l = xc_sharing_used_frames(ctx->xch); > + if ( l < 0 && l != -ENOSYS ) { > + LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, l, > + "getting sharing used frames"); > + return ERROR_FAIL; > + } > + physinfo->sharing_used_frames = (l == -ENOSYS) ? 0 : l; > physinfo->nr_nodes = xcphysinfo.nr_nodes; > memcpy(physinfo->hw_cap,xcphysinfo.hw_cap, sizeof(physinfo->hw_cap)); >
Ian Jackson
2012-Sep-05 14:46 UTC
Re: [PATCH] libxl: handle errors from xc_sharing_* info functions
Ian Campbell writes ("[PATCH] libxl: handle errors from xc_sharing_* info
functions"):> I don''t have a 32 bit system handy, so tested on x86_64 with a
libxc
> hacked to return -ENOSYS and -EINVAL.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> - physinfo->sharing_freed_pages =
xc_sharing_freed_pages(ctx->xch);
> - physinfo->sharing_used_frames =
xc_sharing_used_frames(ctx->xch);
> + l = xc_sharing_freed_pages(ctx->xch);
> + if ( l < 0 && l != -ENOSYS ) {
> + LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, l,
> + "getting sharing freed pages");
> + return ERROR_FAIL;
> + }
> + physinfo->sharing_freed_pages = (l == -ENOSYS) ? 0 : l;
Although you do like doing this the convoluted way. What would have
been wrong with
if (l == -ENOSYS) {
l = 0;
} else if (l < 0) {
LOG...
return ERROR_FAIL;
}
? :-)
Ian.
Ian Campbell
2012-Sep-05 14:52 UTC
Re: [PATCH] libxl: handle errors from xc_sharing_* info functions
On Wed, 2012-09-05 at 15:46 +0100, Ian Jackson wrote:> Ian Campbell writes ("[PATCH] libxl: handle errors from xc_sharing_* info functions"): > > I don''t have a 32 bit system handy, so tested on x86_64 with a libxc > > hacked to return -ENOSYS and -EINVAL. > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> > > > - physinfo->sharing_freed_pages = xc_sharing_freed_pages(ctx->xch); > > - physinfo->sharing_used_frames = xc_sharing_used_frames(ctx->xch); > > + l = xc_sharing_freed_pages(ctx->xch); > > + if ( l < 0 && l != -ENOSYS ) { > > + LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, l, > > + "getting sharing freed pages"); > > + return ERROR_FAIL; > > + } > > + physinfo->sharing_freed_pages = (l == -ENOSYS) ? 0 : l; > > Although you do like doing this the convoluted way. What would have > been wrong with > if (l == -ENOSYS) { > l = 0; > } else if (l < 0) { > LOG... > return ERROR_FAIL; > } > ? :-)Nothing, just in a particular frame of mind I guess. I''ll respin with this approach, it''s less mad.
Ian Campbell
2012-Sep-10 16:26 UTC
Re: [PATCH] libxl: handle errors from xc_sharing_* info functions
On Wed, 2012-09-05 at 15:52 +0100, Ian Campbell wrote:> I''ll respin with this approach, it''s less mad.8<------------------------------------- # HG changeset patch # User Ian Campbell <ian.campbell@citrix.com> # Date 1347294326 -3600 # Node ID e2a8f423ed64c696a61dec88fe8e1976939a7193 # Parent b02d3fdfab4295c4da3594444cabed898fcc90f7 libxl: handle errors from xc_sharing_* info functions On a 32 bit hypervisor xl info currently reports: sharing_freed_memory : 72057594037927935 sharing_used_memory : 72057594037927935 Eat the ENOSYS and turn it into 0. Log and propagate other errors. I don''t have a 32 bit system handy, so tested on x86_64 with a libxc hacked to return -ENOSYS and -EINVAL. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Andres Lagar-Cavilla <andres@lagarcavilla.org> diff -r b02d3fdfab42 -r e2a8f423ed64 tools/libxl/libxl.c --- a/tools/libxl/libxl.c Mon Sep 10 17:22:01 2012 +0100 +++ b/tools/libxl/libxl.c Mon Sep 10 17:25:26 2012 +0100 @@ -3622,6 +3622,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, l { xc_physinfo_t xcphysinfo = { 0 }; int rc; + long l; rc = xc_physinfo(ctx->xch, &xcphysinfo); if (rc != 0) { @@ -3636,8 +3637,24 @@ int libxl_get_physinfo(libxl_ctx *ctx, l physinfo->total_pages = xcphysinfo.total_pages; physinfo->free_pages = xcphysinfo.free_pages; physinfo->scrub_pages = xcphysinfo.scrub_pages; - physinfo->sharing_freed_pages = xc_sharing_freed_pages(ctx->xch); - physinfo->sharing_used_frames = xc_sharing_used_frames(ctx->xch); + l = xc_sharing_freed_pages(ctx->xch); + if (l == -ENOSYS) { + l = 0; + } else if (l < 0) { + LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, l, + "getting sharing freed pages"); + return ERROR_FAIL; + } + physinfo->sharing_freed_pages = l; + l = xc_sharing_used_frames(ctx->xch); + if (l == -ENOSYS) { + l = 0; + } else if (l < 0) { + LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, l, + "getting sharing used frames"); + return ERROR_FAIL; + } + physinfo->sharing_used_frames = l; physinfo->nr_nodes = xcphysinfo.nr_nodes; memcpy(physinfo->hw_cap,xcphysinfo.hw_cap, sizeof(physinfo->hw_cap));
Andres Lagar-Cavilla
2012-Sep-11 14:28 UTC
Re: [PATCH 13/20] arch/x86: Add missing domctl and mem_sharing XSM hooks
The patch looks good but I have one concern. In the hunk below you dropped the cd->is_dying check. Andres> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c > index 5103285..395d302 100644 > --- a/xen/arch/x86/mm/mem_sharing.c > +++ b/xen/arch/x86/mm/mem_sharing.c > @@ -34,6 +34,7 @@ > #include <asm/atomic.h> > #include <xen/rcupdate.h> > #include <asm/event.h> > +#include <xsm/xsm.h> > > #include "mm-locks.h" > > @@ -1345,11 +1346,18 @@ int mem_sharing_memop(struct domain *d, > xen_mem_sharing_op_t *mec) > if ( !mem_sharing_enabled(d) ) > return -EINVAL; > > - cd = get_mem_event_op_target(mec->u.share.client_domain, &rc); > + cd = rcu_lock_domain_by_any_id(mec->u.share.client_domain); > if ( !cd ) > + return -ESRCH; > + > + rc = xsm_mem_sharing_op(d, cd, mec->op); > + if ( rc ) > + { > + rcu_unlock_domain(cd); > return rc; > + } > > - if ( !mem_sharing_enabled(cd) ) > + if ( cd == current->domain || !mem_sharing_enabled(cd) ) > { > rcu_unlock_domain(cd); > return -EINVAL; > @@ -1401,11 +1409,18 @@ int mem_sharing_memop(struct domain *d, > xen_mem_sharing_op_t *mec) > if ( !mem_sharing_enabled(d) ) > return -EINVAL; > > - cd = get_mem_event_op_target(mec->u.share.client_domain, &rc); > + cd = rcu_lock_domain_by_any_id(mec->u.share.client_domain); > if ( !cd ) > + return -ESRCH; > + > + rc = xsm_mem_sharing_op(d, cd, mec->op); > + if ( rc ) > + { > + rcu_unlock_domain(cd); > return rc; > + } > > - if ( !mem_sharing_enabled(cd) ) > + if ( cd == current->domain || !mem_sharing_enabled(cd) ) > { > rcu_unlock_domain(cd); > return -EINVAL; >
Ian Campbell
2012-Sep-14 09:05 UTC
Re: [PATCH] libxl: handle errors from xc_sharing_* info functions
On Mon, 2012-09-10 at 17:26 +0100, Ian Campbell wrote:> libxl: handle errors from xc_sharing_* info functions > > On a 32 bit hypervisor xl info currently reports: > sharing_freed_memory : 72057594037927935 > sharing_used_memory : 72057594037927935 > > Eat the ENOSYS and turn it into 0. Log and propagate other errors. > > I don''t have a 32 bit system handy, so tested on x86_64 with a libxc > hacked to return -ENOSYS and -EINVAL. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> > Acked-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>Applied, thanks.