diff mbox

[v4,05/34] libxc/libxl/python/xenstat: Use new XEN_VERSION_OP hypercall

Message ID 1458064616-23101-6-git-send-email-konrad.wilk@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Konrad Rzeszutek Wilk March 15, 2016, 5:56 p.m. UTC
We change the xen_version libxc code to use the new hypercall.
Which of course means every user in the code base has to
be changed over.

It is important to note that the xc_version_op has a different
return semantic than the previous one. It returns negative
values on error (like the old one), but it also returns
an positive value on success (unlike the old one). The positive
value is the number of bytes copied in.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: David Scott <dave@recoil.org>
Cc: George Dunlap <george.dunlap@eu.citrix.com>

v2: Use xc_version_op_val_t instead of uint32 or such
v3: Make sure to check ret < 0 instead of ret (as it returns the size) -
    in Ocaml code. Found by Andrew.
v4: Update comment for xc_version to mention the return the size
---
 tools/libxc/include/xenctrl.h          | 24 ++++++++++-
 tools/libxc/xc_core.c                  | 35 +++++++--------
 tools/libxc/xc_dom_boot.c              | 12 +++++-
 tools/libxc/xc_domain.c                |  3 +-
 tools/libxc/xc_private.c               | 53 ++++-------------------
 tools/libxc/xc_private.h               |  7 +--
 tools/libxc/xc_resume.c                |  3 +-
 tools/libxc/xc_sr_save.c               |  9 ++--
 tools/libxc/xg_save_restore.h          |  6 ++-
 tools/libxl/libxl.c                    | 79 ++++++++++++++++++++++------------
 tools/ocaml/libs/xc/xenctrl_stubs.c    | 39 +++++++----------
 tools/python/xen/lowlevel/xc/xc.c      | 30 +++++++------
 tools/xenstat/libxenstat/src/xenstat.c | 12 +++---
 tools/xentrace/xenctx.c                |  3 +-
 14 files changed, 169 insertions(+), 146 deletions(-)

Comments

Andrew Cooper March 15, 2016, 6:45 p.m. UTC | #1
On 15/03/16 17:56, Konrad Rzeszutek Wilk wrote:
> We change the xen_version libxc code to use the new hypercall.
> Which of course means every user in the code base has to
> be changed over.
>
> It is important to note that the xc_version_op has a different
> return semantic than the previous one. It returns negative
> values on error (like the old one), but it also returns
> an positive value on success (unlike the old one). The positive
> value is the number of bytes copied in.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: David Scott <dave@recoil.org>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
>
> v2: Use xc_version_op_val_t instead of uint32 or such
> v3: Make sure to check ret < 0 instead of ret (as it returns the size) -
>     in Ocaml code. Found by Andrew.
> v4: Update comment for xc_version to mention the return the size
> ---
>  tools/libxc/include/xenctrl.h          | 24 ++++++++++-
>  tools/libxc/xc_core.c                  | 35 +++++++--------
>  tools/libxc/xc_dom_boot.c              | 12 +++++-
>  tools/libxc/xc_domain.c                |  3 +-
>  tools/libxc/xc_private.c               | 53 ++++-------------------
>  tools/libxc/xc_private.h               |  7 +--
>  tools/libxc/xc_resume.c                |  3 +-
>  tools/libxc/xc_sr_save.c               |  9 ++--
>  tools/libxc/xg_save_restore.h          |  6 ++-
>  tools/libxl/libxl.c                    | 79 ++++++++++++++++++++++------------
>  tools/ocaml/libs/xc/xenctrl_stubs.c    | 39 +++++++----------
>  tools/python/xen/lowlevel/xc/xc.c      | 30 +++++++------
>  tools/xenstat/libxenstat/src/xenstat.c | 12 +++---
>  tools/xentrace/xenctx.c                |  3 +-
>  14 files changed, 169 insertions(+), 146 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 150d727..379de30 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -1477,7 +1477,29 @@ int xc_tbuf_set_evt_mask(xc_interface *xch, uint32_t mask);
>  int xc_domctl(xc_interface *xch, struct xen_domctl *domctl);
>  int xc_sysctl(xc_interface *xch, struct xen_sysctl *sysctl);
>  
> -int xc_version(xc_interface *xch, int cmd, void *arg);
> +/**
> + * This function returns the size of buffer to be allocated for
> + * the cmd. The cmd are XEN_VERSION_OP_*.
> + */

This should return ssize_t as it could also plausibly return -1 and set
errno.

> +int xc_version_len(xc_interface *xch, unsigned int cmd);
> +/**
> + * This function retrieves the information from the version_op hypercall.
> + * The len is the size of the arg buffer. If arg is NULL, will not
> + * perform hypercall - instead will just return the size of arg
> + * buffer that is needed.
> + *
> + * Note that prior to Xen 4.7 this would return 0 for success and
> + * negative value (-1) for error (with the error in errno). In Xen 4.7
> + * and later for success it will return an positive value which is the
> + * number of bytes copied in arg.
> + *
> + * It can also return -1 with various errno values:
> + *  - EPERM - not permitted.
> + *  - ENOBUFS - the len was to short, output in arg truncated.
> + *  - ENOSYS - not implemented.
> + *
> + */
> +int xc_version(xc_interface *xch, unsigned int cmd, void *arg, ssize_t len);

This can get away with taking a size_t len.  I am not sure how much we
care about people trying to claim negative length for *arg.

> diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
> index c40a4e9..23876f0 100644
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -1204,34 +1204,40 @@ static PyObject *pyxc_xeninfo(XcObject *self)
>      xen_capabilities_info_t xen_caps;
>      xen_platform_parameters_t p_parms;
>      xen_commandline_t xen_commandline;
> -    long xen_version;
> -    long xen_pagesize;
> +    xen_version_op_val_t xen_version;
> +    xen_version_op_val_t xen_pagesize;
>      char str[128];
>  
> -    xen_version = xc_version(self->xc_handle, XENVER_version, NULL);
> -
> -    if ( xc_version(self->xc_handle, XENVER_extraversion, &xen_extra) != 0 )
> +    if ( xc_version(self->xc_handle, XEN_VERSION_OP_version, &xen_version,
> +                    sizeof(xen_version)) < 0)

Xen Style.

> --- a/tools/xenstat/libxenstat/src/xenstat.c
> +++ b/tools/xenstat/libxenstat/src/xenstat.c
> @@ -621,20 +621,18 @@ unsigned long long xenstat_network_tdrop(xenstat_network * network)
>  /* Collect Xen version information */
>  static int xenstat_collect_xen_version(xenstat_node * node)
>  {
> -	long vnum = 0;
> +	xen_version_op_val_t vnum = 0;
>  	xen_extraversion_t version;
>  
>  	/* Collect Xen version information if not already collected */
>  	if (node->handle->xen_version[0] == '\0') {
>  		/* Get the Xen version number and extraversion string */
> -		vnum = xc_version(node->handle->xc_handle,
> -			XENVER_version, NULL);
> -
> -		if (vnum < 0)
> +		if (xc_version(node->handle->xc_handle,
> +			           XEN_VERSION_OP_version, &vnum, sizeof(vnum)) < 0 )
>  			return 0;

Curiously, the opposite style bug here.

With these trivial bits fixed, Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>, and a Tested-by: for the Ocaml stubs
(eventually).

~Andrew
George Dunlap March 16, 2016, 12:31 p.m. UTC | #2
On 15/03/16 17:56, Konrad Rzeszutek Wilk wrote:
> We change the xen_version libxc code to use the new hypercall.
> Which of course means every user in the code base has to
> be changed over.
> 
> It is important to note that the xc_version_op has a different
> return semantic than the previous one. It returns negative
> values on error (like the old one), but it also returns
> an positive value on success (unlike the old one). The positive
> value is the number of bytes copied in.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: David Scott <dave@recoil.org>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>

xenctx bit:

Acked-by: George Dunlap <george.dunlap@citrix.com>
Wei Liu March 16, 2016, 6:11 p.m. UTC | #3
On Tue, Mar 15, 2016 at 01:56:27PM -0400, Konrad Rzeszutek Wilk wrote:
[...]
> diff --git a/tools/libxc/xg_save_restore.h b/tools/libxc/xg_save_restore.h
> index 303081d..2663969 100644
> --- a/tools/libxc/xg_save_restore.h
> +++ b/tools/libxc/xg_save_restore.h
> @@ -57,10 +57,12 @@ static inline int get_platform_info(xc_interface *xch, uint32_t dom,
>      xen_capabilities_info_t xen_caps = "";
>      xen_platform_parameters_t xen_params;
>  
> -    if (xc_version(xch, XENVER_platform_parameters, &xen_params) != 0)
> +    if (xc_version(xch, XEN_VERSION_OP_platform_parameters, &xen_params,
> +                   sizeof(xen_params)) < 0)
>          return 0;
>  
> -    if (xc_version(xch, XENVER_capabilities, &xen_caps) != 0)
> +    if (xc_version(xch, XEN_VERSION_OP_capabilities, xen_caps,
> +                   sizeof(xen_caps)) < 0)
>          return 0;
>  
>      if (xc_maximum_ram_page(xch, max_mfn))
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 93e228d..dc660b7 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -5191,50 +5191,73 @@ libxl_numainfo *libxl_get_numainfo(libxl_ctx *ctx, int *nr)
>      return ret;
>  }
>  
> +
> +static int xc_version_wrapper(libxl_ctx *ctx, unsigned int cmd, char *buf, ssize_t len, char **dst)
> +{

This function should accept libxl__gc *gc instead of libxl_ctx.

Preferably is should be renamed to

libxl__xc_version_wrapper

> +    GC_INIT(ctx);
> +    int r;
> +
> +    r = xc_version(ctx->xch, cmd, buf, len);

Then here CTX->xch

> +    if ( r == -EPERM )
> +        buf[0] = '\0';
> +    else if ( r < 0 )
> +    {
> +        GC_FREE;
> +        return r;
> +    }
> +    *dst = libxl__strdup(NOGC, buf);
> +    GC_FREE;

Then get rid of all GC_* macros.

> +    return 0;
> +}
> +
>  const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
[...]
> diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
> index c40a4e9..23876f0 100644
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -1204,34 +1204,40 @@ static PyObject *pyxc_xeninfo(XcObject *self)
>      xen_capabilities_info_t xen_caps;
>      xen_platform_parameters_t p_parms;
>      xen_commandline_t xen_commandline;
> -    long xen_version;
> -    long xen_pagesize;
> +    xen_version_op_val_t xen_version;
> +    xen_version_op_val_t xen_pagesize;
>      char str[128];
>  
> -    xen_version = xc_version(self->xc_handle, XENVER_version, NULL);
> -
> -    if ( xc_version(self->xc_handle, XENVER_extraversion, &xen_extra) != 0 )
> +    if ( xc_version(self->xc_handle, XEN_VERSION_OP_version, &xen_version,
> +                    sizeof(xen_version)) < 0)
>          return pyxc_error_to_exception(self->xc_handle);
>  
> -    if ( xc_version(self->xc_handle, XENVER_compile_info, &xen_cc) != 0 )
> +    if ( xc_version(self->xc_handle, XEN_VERSION_OP_extraversion, &xen_extra,
> +                    sizeof(xen_extra)) < 0 )
>          return pyxc_error_to_exception(self->xc_handle);
>  
> -    if ( xc_version(self->xc_handle, XENVER_changeset, &xen_chgset) != 0 )
> +    memset(&xen_cc, 0, sizeof(xen_cc));
> +
> +    if ( xc_version(self->xc_handle, XEN_VERSION_OP_changeset, &xen_chgset,
> +                    sizeof(xen_chgset)) < 0 )
>          return pyxc_error_to_exception(self->xc_handle);
>  
> -    if ( xc_version(self->xc_handle, XENVER_capabilities, &xen_caps) != 0 )
> +    if ( xc_version(self->xc_handle, XEN_VERSION_OP_capabilities, &xen_caps,
> +                   sizeof(xen_caps)) < 0 )

Indentation.

>          return pyxc_error_to_exception(self->xc_handle);
>  
> -    if ( xc_version(self->xc_handle, XENVER_platform_parameters, &p_parms) != 0 )
> +    if ( xc_version(self->xc_handle, XEN_VERSION_OP_platform_parameters,
> +                    &p_parms, sizeof(p_parms)) < 0 )
>          return pyxc_error_to_exception(self->xc_handle);
>  
> -    if ( xc_version(self->xc_handle, XENVER_commandline, &xen_commandline) != 0 )
> +    if ( xc_version(self->xc_handle, XEN_VERSION_OP_commandline,
> +                    &xen_commandline, sizeof(xen_commandline)) < 0 )
>          return pyxc_error_to_exception(self->xc_handle);
>  
>      snprintf(str, sizeof(str), "virt_start=0x%"PRI_xen_ulong, p_parms.virt_start);
>  
> -    xen_pagesize = xc_version(self->xc_handle, XENVER_pagesize, NULL);
> -    if (xen_pagesize < 0 )
> +    if ( xc_version(self->xc_handle, XEN_VERSION_OP_pagesize, &xen_pagesize,
> +                    sizeof(xen_pagesize)) < 0)
>          return pyxc_error_to_exception(self->xc_handle);
>  
>      return Py_BuildValue("{s:i,s:i,s:s,s:s,s:i,s:s,s:s,s:s,s:s,s:s,s:s,s:s}",
> diff --git a/tools/xenstat/libxenstat/src/xenstat.c b/tools/xenstat/libxenstat/src/xenstat.c
> index 3495f3f..723e46a 100644
> --- a/tools/xenstat/libxenstat/src/xenstat.c
> +++ b/tools/xenstat/libxenstat/src/xenstat.c
> @@ -621,20 +621,18 @@ unsigned long long xenstat_network_tdrop(xenstat_network * network)
>  /* Collect Xen version information */
>  static int xenstat_collect_xen_version(xenstat_node * node)
>  {
> -	long vnum = 0;
> +	xen_version_op_val_t vnum = 0;
>  	xen_extraversion_t version;
>  
>  	/* Collect Xen version information if not already collected */
>  	if (node->handle->xen_version[0] == '\0') {
>  		/* Get the Xen version number and extraversion string */
> -		vnum = xc_version(node->handle->xc_handle,
> -			XENVER_version, NULL);
> -
> -		if (vnum < 0)
> +		if (xc_version(node->handle->xc_handle,
> +			           XEN_VERSION_OP_version, &vnum, sizeof(vnum)) < 0 )

Indentation.

>  			return 0;
>  
> -		if (xc_version(node->handle->xc_handle, XENVER_extraversion,
> -			&version) < 0)
> +		if (xc_version(node->handle->xc_handle, XEN_VERSION_OP_extraversion,
> +			           &version, sizeof(version)) < 0)

Indentation.


Wei.

>  			return 0;
>  		/* Format the version information as a string and store it */
>  		snprintf(node->handle->xen_version, VERSION_SIZE, "%ld.%ld%s",
> diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
> index e647179..14d2f8b 100644
> --- a/tools/xentrace/xenctx.c
> +++ b/tools/xentrace/xenctx.c
> @@ -1000,7 +1000,8 @@ static void dump_ctx(int vcpu)
>              guest_word_size = (cpuctx.msr_efer & 0x400) ? 8 :
>                  guest_protected_mode ? 4 : 2;
>              /* HVM guest context records are always host-sized */
> -            if (xc_version(xenctx.xc_handle, XENVER_capabilities, &xen_caps) != 0) {
> +            if (xc_version(xenctx.xc_handle, XEN_VERSION_OP_capabilities,
> +                           &xen_caps, sizeof(xen_caps)) < 0) {
>                  perror("xc_version");
>                  return;
>              }
> -- 
> 2.5.0
>
diff mbox

Patch

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 150d727..379de30 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1477,7 +1477,29 @@  int xc_tbuf_set_evt_mask(xc_interface *xch, uint32_t mask);
 int xc_domctl(xc_interface *xch, struct xen_domctl *domctl);
 int xc_sysctl(xc_interface *xch, struct xen_sysctl *sysctl);
 
-int xc_version(xc_interface *xch, int cmd, void *arg);
+/**
+ * This function returns the size of buffer to be allocated for
+ * the cmd. The cmd are XEN_VERSION_OP_*.
+ */
+int xc_version_len(xc_interface *xch, unsigned int cmd);
+/**
+ * This function retrieves the information from the version_op hypercall.
+ * The len is the size of the arg buffer. If arg is NULL, will not
+ * perform hypercall - instead will just return the size of arg
+ * buffer that is needed.
+ *
+ * Note that prior to Xen 4.7 this would return 0 for success and
+ * negative value (-1) for error (with the error in errno). In Xen 4.7
+ * and later for success it will return an positive value which is the
+ * number of bytes copied in arg.
+ *
+ * It can also return -1 with various errno values:
+ *  - EPERM - not permitted.
+ *  - ENOBUFS - the len was to short, output in arg truncated.
+ *  - ENOSYS - not implemented.
+ *
+ */
+int xc_version(xc_interface *xch, unsigned int cmd, void *arg, ssize_t len);
 
 int xc_flask_op(xc_interface *xch, xen_flask_op_t *op);
 
diff --git a/tools/libxc/xc_core.c b/tools/libxc/xc_core.c
index d792566..58b03d6 100644
--- a/tools/libxc/xc_core.c
+++ b/tools/libxc/xc_core.c
@@ -270,42 +270,43 @@  elfnote_fill_xen_version(xc_interface *xch,
                          *xen_version)
 {
     int rc;
+    xen_version_op_val_t val = 0;
     memset(xen_version, 0, sizeof(*xen_version));
 
-    rc = xc_version(xch, XENVER_version, NULL);
+    rc = xc_version(xch, XEN_VERSION_OP_version, &val, sizeof(val));
     if ( rc < 0 )
         return rc;
-    xen_version->major_version = rc >> 16;
-    xen_version->minor_version = rc & ((1 << 16) - 1);
+    xen_version->major_version = val >> 16;
+    xen_version->minor_version = val & ((1 << 16) - 1);
 
-    rc = xc_version(xch, XENVER_extraversion,
-                    &xen_version->extra_version);
+    rc = xc_version(xch, XEN_VERSION_OP_extraversion,
+                    xen_version->extra_version,
+                    sizeof(xen_version->extra_version));
     if ( rc < 0 )
         return rc;
 
-    rc = xc_version(xch, XENVER_compile_info,
-                    &xen_version->compile_info);
+    rc = xc_version(xch, XEN_VERSION_OP_capabilities,
+                    xen_version->capabilities,
+                    sizeof(xen_version->capabilities));
     if ( rc < 0 )
         return rc;
 
-    rc = xc_version(xch,
-                    XENVER_capabilities, &xen_version->capabilities);
+    rc = xc_version(xch, XEN_VERSION_OP_changeset, xen_version->changeset,
+                    sizeof(xen_version->changeset));
     if ( rc < 0 )
         return rc;
 
-    rc = xc_version(xch, XENVER_changeset, &xen_version->changeset);
+    rc = xc_version(xch, XEN_VERSION_OP_platform_parameters,
+                    &xen_version->platform_parameters,
+                    sizeof(xen_version->platform_parameters));
     if ( rc < 0 )
         return rc;
 
-    rc = xc_version(xch, XENVER_platform_parameters,
-                    &xen_version->platform_parameters);
+    val = 0;
+    rc = xc_version(xch, XEN_VERSION_OP_pagesize, &val, sizeof(val));
     if ( rc < 0 )
         return rc;
-
-    rc = xc_version(xch, XENVER_pagesize, NULL);
-    if ( rc < 0 )
-        return rc;
-    xen_version->pagesize = rc;
+    xen_version->pagesize = val;
 
     return 0;
 }
diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
index 791041b..3f65095 100644
--- a/tools/libxc/xc_dom_boot.c
+++ b/tools/libxc/xc_dom_boot.c
@@ -112,11 +112,19 @@  int xc_dom_compat_check(struct xc_dom_image *dom)
 
 int xc_dom_boot_xen_init(struct xc_dom_image *dom, xc_interface *xch, domid_t domid)
 {
+    xen_version_op_val_t val = 0;
+
+    if ( xc_version(xch, XEN_VERSION_OP_version, &val, sizeof(val)) < 0 )
+    {
+        xc_dom_panic(xch, XC_INTERNAL_ERROR, "can't get Xen version!");
+        return -1;
+    }
+    dom->xen_version = val;
     dom->xch = xch;
     dom->guest_domid = domid;
 
-    dom->xen_version = xc_version(xch, XENVER_version, NULL);
-    if ( xc_version(xch, XENVER_capabilities, &dom->xen_caps) < 0 )
+    if ( xc_version(xch, XEN_VERSION_OP_capabilities, dom->xen_caps,
+                    sizeof(dom->xen_caps)) < 0 )
     {
         xc_dom_panic(xch, XC_INTERNAL_ERROR, "can't get xen capabilities");
         return -1;
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 050216e..c214700 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -2084,7 +2084,8 @@  int xc_map_domain_meminfo(xc_interface *xch, int domid,
     _di.guest_width = minfo->guest_width;
 
     /* Get page table levels (see get_platform_info() in xg_save_restore.h */
-    if ( xc_version(xch, XENVER_capabilities, &xen_caps) )
+    if ( xc_version(xch, XEN_VERSION_OP_capabilities, xen_caps,
+                    sizeof(xen_caps)) < 0 )
     {
         PERROR("Could not get Xen capabilities (for page table levels)");
         return -1;
diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
index c41e433..e33ad8f 100644
--- a/tools/libxc/xc_private.c
+++ b/tools/libxc/xc_private.c
@@ -457,58 +457,23 @@  int xc_sysctl(xc_interface *xch, struct xen_sysctl *sysctl)
     return do_sysctl(xch, sysctl);
 }
 
-int xc_version(xc_interface *xch, int cmd, void *arg)
+int xc_version_len(xc_interface *xch, unsigned int cmd)
 {
-    DECLARE_HYPERCALL_BOUNCE(arg, 0, XC_HYPERCALL_BUFFER_BOUNCE_OUT); /* Size unknown until cmd decoded */
-    size_t sz;
-    int rc;
-
-    switch ( cmd )
-    {
-    case XENVER_version:
-        sz = 0;
-        break;
-    case XENVER_extraversion:
-        sz = sizeof(xen_extraversion_t);
-        break;
-    case XENVER_compile_info:
-        sz = sizeof(xen_compile_info_t);
-        break;
-    case XENVER_capabilities:
-        sz = sizeof(xen_capabilities_info_t);
-        break;
-    case XENVER_changeset:
-        sz = sizeof(xen_changeset_info_t);
-        break;
-    case XENVER_platform_parameters:
-        sz = sizeof(xen_platform_parameters_t);
-        break;
-    case XENVER_get_features:
-        sz = sizeof(xen_feature_info_t);
-        break;
-    case XENVER_pagesize:
-        sz = 0;
-        break;
-    case XENVER_guest_handle:
-        sz = sizeof(xen_domain_handle_t);
-        break;
-    case XENVER_commandline:
-        sz = sizeof(xen_commandline_t);
-        break;
-    default:
-        ERROR("xc_version: unknown command %d\n", cmd);
-        return -EINVAL;
-    }
+    return do_version_op(xch, cmd, NULL, 0);
+}
 
-    HYPERCALL_BOUNCE_SET_SIZE(arg, sz);
+int xc_version(xc_interface *xch, unsigned int cmd, void *arg, ssize_t sz)
+{
+    DECLARE_HYPERCALL_BOUNCE(arg, sz, XC_HYPERCALL_BUFFER_BOUNCE_OUT);
+    int rc;
 
-    if ( (sz != 0) && xc_hypercall_bounce_pre(xch, arg) )
+    if ( xc_hypercall_bounce_pre(xch, arg) )
     {
         PERROR("Could not bounce buffer for version hypercall");
         return -ENOMEM;
     }
 
-    rc = do_xen_version(xch, cmd, HYPERCALL_BUFFER(arg));
+    rc = do_version_op(xch, cmd, HYPERCALL_BUFFER(arg), sz);
 
     if ( sz != 0 )
         xc_hypercall_bounce_post(xch, arg);
diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
index aa8daf1..5be8fdd 100644
--- a/tools/libxc/xc_private.h
+++ b/tools/libxc/xc_private.h
@@ -214,11 +214,12 @@  void xc__hypercall_buffer_cache_release(xc_interface *xch);
  * Hypercall interfaces.
  */
 
-static inline int do_xen_version(xc_interface *xch, int cmd, xc_hypercall_buffer_t *dest)
+static inline long do_version_op(xc_interface *xch, int cmd,
+                                 xc_hypercall_buffer_t *dest, ssize_t len)
 {
     DECLARE_HYPERCALL_BUFFER_ARGUMENT(dest);
-    return xencall2(xch->xcall, __HYPERVISOR_xen_version,
-                    cmd, HYPERCALL_BUFFER_AS_ARG(dest));
+    return xencall3(xch->xcall, __HYPERVISOR_version_op,
+                    cmd, HYPERCALL_BUFFER_AS_ARG(dest), len);
 }
 
 static inline int do_physdev_op(xc_interface *xch, int cmd, void *op, size_t len)
diff --git a/tools/libxc/xc_resume.c b/tools/libxc/xc_resume.c
index e692b81..7dfc3da 100644
--- a/tools/libxc/xc_resume.c
+++ b/tools/libxc/xc_resume.c
@@ -56,7 +56,8 @@  static int modify_returncode(xc_interface *xch, uint32_t domid)
             return 0;
 
         /* HVM guests have host address width. */
-        if ( xc_version(xch, XENVER_capabilities, &caps) != 0 )
+        if ( xc_version(xch, XEN_VERSION_OP_capabilities, caps,
+                        sizeof(caps)) < 0 )
         {
             PERROR("Could not get Xen capabilities");
             return -1;
diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index e258b7c..6daafc4 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -9,7 +9,7 @@ 
 static int write_headers(struct xc_sr_context *ctx, uint16_t guest_type)
 {
     xc_interface *xch = ctx->xch;
-    int32_t xen_version = xc_version(xch, XENVER_version, NULL);
+    xen_version_op_val_t xen_version;
     struct xc_sr_ihdr ihdr =
         {
             .marker  = IHDR_MARKER,
@@ -21,15 +21,16 @@  static int write_headers(struct xc_sr_context *ctx, uint16_t guest_type)
         {
             .type       = guest_type,
             .page_shift = XC_PAGE_SHIFT,
-            .xen_major  = (xen_version >> 16) & 0xffff,
-            .xen_minor  = (xen_version)       & 0xffff,
         };
 
-    if ( xen_version < 0 )
+    if ( xc_version(xch, XEN_VERSION_OP_version, &xen_version,
+                    sizeof(xen_version)) < 0 )
     {
         PERROR("Unable to obtain Xen Version");
         return -1;
     }
+    dhdr.xen_major = (xen_version >> 16) & 0xffff;
+    dhdr.xen_minor = (xen_version)       & 0xffff;
 
     if ( write_exact(ctx->fd, &ihdr, sizeof(ihdr)) )
     {
diff --git a/tools/libxc/xg_save_restore.h b/tools/libxc/xg_save_restore.h
index 303081d..2663969 100644
--- a/tools/libxc/xg_save_restore.h
+++ b/tools/libxc/xg_save_restore.h
@@ -57,10 +57,12 @@  static inline int get_platform_info(xc_interface *xch, uint32_t dom,
     xen_capabilities_info_t xen_caps = "";
     xen_platform_parameters_t xen_params;
 
-    if (xc_version(xch, XENVER_platform_parameters, &xen_params) != 0)
+    if (xc_version(xch, XEN_VERSION_OP_platform_parameters, &xen_params,
+                   sizeof(xen_params)) < 0)
         return 0;
 
-    if (xc_version(xch, XENVER_capabilities, &xen_caps) != 0)
+    if (xc_version(xch, XEN_VERSION_OP_capabilities, xen_caps,
+                   sizeof(xen_caps)) < 0)
         return 0;
 
     if (xc_maximum_ram_page(xch, max_mfn))
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 93e228d..dc660b7 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5191,50 +5191,73 @@  libxl_numainfo *libxl_get_numainfo(libxl_ctx *ctx, int *nr)
     return ret;
 }
 
+
+static int xc_version_wrapper(libxl_ctx *ctx, unsigned int cmd, char *buf, ssize_t len, char **dst)
+{
+    GC_INIT(ctx);
+    int r;
+
+    r = xc_version(ctx->xch, cmd, buf, len);
+    if ( r == -EPERM )
+        buf[0] = '\0';
+    else if ( r < 0 )
+    {
+        GC_FREE;
+        return r;
+    }
+    *dst = libxl__strdup(NOGC, buf);
+    GC_FREE;
+    return 0;
+}
+
 const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
 {
     GC_INIT(ctx);
-    union {
-        xen_extraversion_t xen_extra;
-        xen_compile_info_t xen_cc;
-        xen_changeset_info_t xen_chgset;
-        xen_capabilities_info_t xen_caps;
-        xen_platform_parameters_t p_parms;
-        xen_commandline_t xen_commandline;
-    } u;
-    long xen_version;
+    char *buf;
+    xen_version_op_val_t val = 0;
     libxl_version_info *info = &ctx->version_info;
 
     if (info->xen_version_extra != NULL)
         goto out;
 
-    xen_version = xc_version(ctx->xch, XENVER_version, NULL);
-    info->xen_version_major = xen_version >> 16;
-    info->xen_version_minor = xen_version & 0xFF;
+    if (xc_version(ctx->xch, XEN_VERSION_OP_pagesize, &val, sizeof(val)) < 0)
+        goto out;
 
-    xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra);
-    info->xen_version_extra = libxl__strdup(NOGC, u.xen_extra);
+    info->pagesize = val;
+    /* 4K buffer. */
+    buf = libxl__zalloc(gc, info->pagesize);
 
-    xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc);
-    info->compiler = libxl__strdup(NOGC, u.xen_cc.compiler);
-    info->compile_by = libxl__strdup(NOGC, u.xen_cc.compile_by);
-    info->compile_domain = libxl__strdup(NOGC, u.xen_cc.compile_domain);
-    info->compile_date = libxl__strdup(NOGC, u.xen_cc.compile_date);
+    val = 0;
+    if (xc_version(ctx->xch, XEN_VERSION_OP_version, &val, sizeof(val)) < 0)
+        goto out;
+    info->xen_version_major = val >> 16;
+    info->xen_version_minor = val & 0xFF;
 
-    xc_version(ctx->xch, XENVER_capabilities, &u.xen_caps);
-    info->capabilities = libxl__strdup(NOGC, u.xen_caps);
+    if (xc_version_wrapper(ctx, XEN_VERSION_OP_extraversion, buf,
+                           info->pagesize, &info->xen_version_extra) < 0)
+        goto out;
 
-    xc_version(ctx->xch, XENVER_changeset, &u.xen_chgset);
-    info->changeset = libxl__strdup(NOGC, u.xen_chgset);
+    info->compiler = libxl__strdup(NOGC, "");
+    info->compile_by = libxl__strdup(NOGC, "");
+    info->compile_domain = libxl__strdup(NOGC, "");
+    info->compile_date = libxl__strdup(NOGC, "");
 
-    xc_version(ctx->xch, XENVER_platform_parameters, &u.p_parms);
-    info->virt_start = u.p_parms.virt_start;
+    if (xc_version_wrapper(ctx, XEN_VERSION_OP_capabilities, buf,
+                           info->pagesize, &info->capabilities) < 0)
+        goto out;
 
-    info->pagesize = xc_version(ctx->xch, XENVER_pagesize, NULL);
+    if (xc_version_wrapper(ctx, XEN_VERSION_OP_changeset, buf,
+                           info->pagesize, &info->changeset) < 0)
+        goto out;
 
-    xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline);
-    info->commandline = libxl__strdup(NOGC, u.xen_commandline);
+    val = 0;
+    if (xc_version(ctx->xch, XEN_VERSION_OP_platform_parameters, &val,
+                   sizeof(val)) < 0)
+        goto out;
+    info->virt_start = val;
 
+    (void)xc_version_wrapper(ctx, XEN_VERSION_OP_commandline, buf,
+                             info->pagesize, &info->commandline);
  out:
     GC_FREE;
     return info;
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
index 74928e9..623b2d7 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -853,21 +853,21 @@  CAMLprim value stub_xc_version_version(value xch)
 	CAMLparam1(xch);
 	CAMLlocal1(result);
 	xen_extraversion_t extra;
-	long packed;
+	xen_version_op_val_t packed;
 	int retval;
 
 	caml_enter_blocking_section();
-	packed = xc_version(_H(xch), XENVER_version, NULL);
+	retval = xc_version(_H(xch), XEN_VERSION_OP_version, &packed, sizeof(packed));
 	caml_leave_blocking_section();
 
-	if (packed < 0)
+	if (retval < 0)
 		failwith_xc(_H(xch));
 
 	caml_enter_blocking_section();
-	retval = xc_version(_H(xch), XENVER_extraversion, &extra);
+	retval = xc_version(_H(xch), XEN_VERSION_OP_extraversion, &extra, sizeof(extra));
 	caml_leave_blocking_section();
 
-	if (retval)
+	if (retval < 0)
 		failwith_xc(_H(xch));
 
 	result = caml_alloc_tuple(3);
@@ -884,37 +884,28 @@  CAMLprim value stub_xc_version_compile_info(value xch)
 {
 	CAMLparam1(xch);
 	CAMLlocal1(result);
-	xen_compile_info_t ci;
-	int retval;
-
-	caml_enter_blocking_section();
-	retval = xc_version(_H(xch), XENVER_compile_info, &ci);
-	caml_leave_blocking_section();
-
-	if (retval)
-		failwith_xc(_H(xch));
 
 	result = caml_alloc_tuple(4);
 
-	Store_field(result, 0, caml_copy_string(ci.compiler));
-	Store_field(result, 1, caml_copy_string(ci.compile_by));
-	Store_field(result, 2, caml_copy_string(ci.compile_domain));
-	Store_field(result, 3, caml_copy_string(ci.compile_date));
+	Store_field(result, 0, caml_copy_string(""));
+	Store_field(result, 1, caml_copy_string(""));
+	Store_field(result, 2, caml_copy_string(""));
+	Store_field(result, 3, caml_copy_string(""));
 
 	CAMLreturn(result);
 }
 
 
-static value xc_version_single_string(value xch, int code, void *info)
+static value xc_version_single_string(value xch, int code, void *info, ssize_t len)
 {
 	CAMLparam1(xch);
 	int retval;
 
 	caml_enter_blocking_section();
-	retval = xc_version(_H(xch), code, info);
+	retval = xc_version(_H(xch), code, info, len);
 	caml_leave_blocking_section();
 
-	if (retval)
+	if (retval < 0)
 		failwith_xc(_H(xch));
 
 	CAMLreturn(caml_copy_string((char *)info));
@@ -925,7 +916,8 @@  CAMLprim value stub_xc_version_changeset(value xch)
 {
 	xen_changeset_info_t ci;
 
-	return xc_version_single_string(xch, XENVER_changeset, &ci);
+	return xc_version_single_string(xch, XEN_VERSION_OP_changeset,
+                                    &ci, sizeof(ci));
 }
 
 
@@ -933,7 +925,8 @@  CAMLprim value stub_xc_version_capabilities(value xch)
 {
 	xen_capabilities_info_t ci;
 
-	return xc_version_single_string(xch, XENVER_capabilities, &ci);
+	return xc_version_single_string(xch, XEN_VERSION_OP_capabilities,
+                                    &ci, sizeof(ci));
 }
 
 
diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index c40a4e9..23876f0 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -1204,34 +1204,40 @@  static PyObject *pyxc_xeninfo(XcObject *self)
     xen_capabilities_info_t xen_caps;
     xen_platform_parameters_t p_parms;
     xen_commandline_t xen_commandline;
-    long xen_version;
-    long xen_pagesize;
+    xen_version_op_val_t xen_version;
+    xen_version_op_val_t xen_pagesize;
     char str[128];
 
-    xen_version = xc_version(self->xc_handle, XENVER_version, NULL);
-
-    if ( xc_version(self->xc_handle, XENVER_extraversion, &xen_extra) != 0 )
+    if ( xc_version(self->xc_handle, XEN_VERSION_OP_version, &xen_version,
+                    sizeof(xen_version)) < 0)
         return pyxc_error_to_exception(self->xc_handle);
 
-    if ( xc_version(self->xc_handle, XENVER_compile_info, &xen_cc) != 0 )
+    if ( xc_version(self->xc_handle, XEN_VERSION_OP_extraversion, &xen_extra,
+                    sizeof(xen_extra)) < 0 )
         return pyxc_error_to_exception(self->xc_handle);
 
-    if ( xc_version(self->xc_handle, XENVER_changeset, &xen_chgset) != 0 )
+    memset(&xen_cc, 0, sizeof(xen_cc));
+
+    if ( xc_version(self->xc_handle, XEN_VERSION_OP_changeset, &xen_chgset,
+                    sizeof(xen_chgset)) < 0 )
         return pyxc_error_to_exception(self->xc_handle);
 
-    if ( xc_version(self->xc_handle, XENVER_capabilities, &xen_caps) != 0 )
+    if ( xc_version(self->xc_handle, XEN_VERSION_OP_capabilities, &xen_caps,
+                   sizeof(xen_caps)) < 0 )
         return pyxc_error_to_exception(self->xc_handle);
 
-    if ( xc_version(self->xc_handle, XENVER_platform_parameters, &p_parms) != 0 )
+    if ( xc_version(self->xc_handle, XEN_VERSION_OP_platform_parameters,
+                    &p_parms, sizeof(p_parms)) < 0 )
         return pyxc_error_to_exception(self->xc_handle);
 
-    if ( xc_version(self->xc_handle, XENVER_commandline, &xen_commandline) != 0 )
+    if ( xc_version(self->xc_handle, XEN_VERSION_OP_commandline,
+                    &xen_commandline, sizeof(xen_commandline)) < 0 )
         return pyxc_error_to_exception(self->xc_handle);
 
     snprintf(str, sizeof(str), "virt_start=0x%"PRI_xen_ulong, p_parms.virt_start);
 
-    xen_pagesize = xc_version(self->xc_handle, XENVER_pagesize, NULL);
-    if (xen_pagesize < 0 )
+    if ( xc_version(self->xc_handle, XEN_VERSION_OP_pagesize, &xen_pagesize,
+                    sizeof(xen_pagesize)) < 0)
         return pyxc_error_to_exception(self->xc_handle);
 
     return Py_BuildValue("{s:i,s:i,s:s,s:s,s:i,s:s,s:s,s:s,s:s,s:s,s:s,s:s}",
diff --git a/tools/xenstat/libxenstat/src/xenstat.c b/tools/xenstat/libxenstat/src/xenstat.c
index 3495f3f..723e46a 100644
--- a/tools/xenstat/libxenstat/src/xenstat.c
+++ b/tools/xenstat/libxenstat/src/xenstat.c
@@ -621,20 +621,18 @@  unsigned long long xenstat_network_tdrop(xenstat_network * network)
 /* Collect Xen version information */
 static int xenstat_collect_xen_version(xenstat_node * node)
 {
-	long vnum = 0;
+	xen_version_op_val_t vnum = 0;
 	xen_extraversion_t version;
 
 	/* Collect Xen version information if not already collected */
 	if (node->handle->xen_version[0] == '\0') {
 		/* Get the Xen version number and extraversion string */
-		vnum = xc_version(node->handle->xc_handle,
-			XENVER_version, NULL);
-
-		if (vnum < 0)
+		if (xc_version(node->handle->xc_handle,
+			           XEN_VERSION_OP_version, &vnum, sizeof(vnum)) < 0 )
 			return 0;
 
-		if (xc_version(node->handle->xc_handle, XENVER_extraversion,
-			&version) < 0)
+		if (xc_version(node->handle->xc_handle, XEN_VERSION_OP_extraversion,
+			           &version, sizeof(version)) < 0)
 			return 0;
 		/* Format the version information as a string and store it */
 		snprintf(node->handle->xen_version, VERSION_SIZE, "%ld.%ld%s",
diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index e647179..14d2f8b 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -1000,7 +1000,8 @@  static void dump_ctx(int vcpu)
             guest_word_size = (cpuctx.msr_efer & 0x400) ? 8 :
                 guest_protected_mode ? 4 : 2;
             /* HVM guest context records are always host-sized */
-            if (xc_version(xenctx.xc_handle, XENVER_capabilities, &xen_caps) != 0) {
+            if (xc_version(xenctx.xc_handle, XEN_VERSION_OP_capabilities,
+                           &xen_caps, sizeof(xen_caps)) < 0) {
                 perror("xc_version");
                 return;
             }