[PATCH] x86/domctl: Improve XEN_DOMCTL_hypercall_init error handling

[PATCH] x86/domctl: Improve XEN_DOMCTL_hypercall_init error handling

[Andrew Cooper]

EACCES cannot be distinguished against an incorrect DOMCTL_INTERFACE_VERSION,
and will cause an incorrect "need to rebuild the user-space tool set?" message
from libxc.  EINVAL is a perfectly reasonable alternative.

On the libxc side, put the useful piece of information in the error message,
rathe than the -1 from do_domctl().

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxc/xc_dom_boot.c |    6 +++---
 xen/arch/x86/domctl.c     |    4 +---
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
index a141eb5..f82db2d 100644
--- a/tools/libxc/xc_dom_boot.c
+++ b/tools/libxc/xc_dom_boot.c
@@ -57,9 +57,9 @@ static int setup_hypercall_page(struct xc_dom_image *dom)
     domctl.u.hypercall_init.gmfn = xc_dom_p2m_guest(dom, pfn);
     rc = do_domctl(dom->xch, &domctl);
     if ( rc != 0 )
-        xc_dom_panic(dom->xch,
-                     XC_INTERNAL_ERROR, "%s: HYPERCALL_INIT failed (rc=%d)",
-                     __FUNCTION__, rc);
+        xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
+                     "%s: HYPERCALL_INIT failed: %d - %s)",
+                     __FUNCTION__, errno, strerror(errno));
     return rc;
 }
 
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 3e5bef1..3ffb083 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -386,16 +386,14 @@ long arch_do_domctl(
 
         page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
 
-        ret = -EACCES;
         if ( !page || !get_page_type(page, PGT_writable_page) )
         {
+            ret = -EINVAL;
             if ( page )
                 put_page(page);
             break;
         }
 
-        ret = 0;
-
         hypercall_page = __map_domain_page(page);
         hypercall_page_initialise(d, hypercall_page);
         unmap_domain_page(hypercall_page);
-- 
1.7.10.4

Re: [PATCH] x86/domctl: Improve XEN_DOMCTL_hypercall_init error handling

[Jan Beulich]

>>> On 11.03.15 at 13:56, <andrew.cooper3@citrix.com> wrote:
> EACCES cannot be distinguished against an incorrect DOMCTL_INTERFACE_VERSION,
> and will cause an incorrect "need to rebuild the user-space tool set?" 
> message
> from libxc.  EINVAL is a perfectly reasonable alternative.

I'm okay with this for the !page case, but I think -EPERM or some
such would be better suited when the page is non-writable but
otherwise valid.

Jan

[PATCH v2 1/6] tools/libxl: Introduce min and max macros

[Andrew Cooper]

This is the same set used by libxc.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>

---
v2: Don't use reserved identifiers in min_t/max_t
---
 tools/libxl/libxl_internal.h |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 934465a..fcbec7f 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -108,6 +108,22 @@
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
 
+#define min(X, Y) ({                             \
+            const typeof (X) _x = (X);           \
+            const typeof (Y) _y = (Y);           \
+            (void) (&_x == &_y);                 \
+            (_x < _y) ? _x : _y; })
+#define max(X, Y) ({                             \
+            const typeof (X) _x = (X);           \
+            const typeof (Y) _y = (Y);           \
+            (void) (&_x == &_y);                 \
+            (_x > _y) ? _x : _y; })
+
+#define min_t(type, x, y)                                               \
+    ({ const type _x = (x); const type _y = (y); _x < _y ? _x: _y; })
+#define max_t(type, x, y)                                               \
+    ({ const type _x = (x); const type _y = (y); _x > _y ? _x: _y; })
+
 #define LIBXL__LOGGING_ENABLED
 
 #ifdef LIBXL__LOGGING_ENABLED
-- 
1.7.10.4

Re: [PATCH v2 1/6] tools/libxl: Introduce min and max macros

[Andrew Cooper]

On 12/03/15 11:01, Andrew Cooper wrote:
> This is the same set used by libxc.
>

Please ignore this - I had an accident with git-send-email.

~Andrew

[PATCH v2] x86/domctl: Improve XEN_DOMCTL_hypercall_init error handling

[Andrew Cooper]

EACCES cannot be distinguished against an incorrect DOMCTL_INTERFACE_VERSION,
and will cause an incorrect "need to rebuild the user-space tool set?" message
from libxc.

On the libxc side, put the useful piece of information in the error message,
rathe than the -1 from do_domctl().

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>

---
v2: Split error between "no such frame" and "bad type"
---
 tools/libxc/xc_dom_boot.c |    6 +++---
 xen/arch/x86/domctl.c     |    8 +++++---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
index a141eb5..f82db2d 100644
--- a/tools/libxc/xc_dom_boot.c
+++ b/tools/libxc/xc_dom_boot.c
@@ -57,9 +57,9 @@ static int setup_hypercall_page(struct xc_dom_image *dom)
     domctl.u.hypercall_init.gmfn = xc_dom_p2m_guest(dom, pfn);
     rc = do_domctl(dom->xch, &domctl);
     if ( rc != 0 )
-        xc_dom_panic(dom->xch,
-                     XC_INTERNAL_ERROR, "%s: HYPERCALL_INIT failed (rc=%d)",
-                     __FUNCTION__, rc);
+        xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
+                     "%s: HYPERCALL_INIT failed: %d - %s)",
+                     __FUNCTION__, errno, strerror(errno));
     return rc;
 }
 
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 3e5bef1..d4f6ccf 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -386,16 +386,18 @@ long arch_do_domctl(
 
         page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
 
-        ret = -EACCES;
         if ( !page || !get_page_type(page, PGT_writable_page) )
         {
             if ( page )
+            {
+                ret = -EPERM;
                 put_page(page);
+            }
+            else
+                ret = -EINVAL;
             break;
         }
 
-        ret = 0;
-
         hypercall_page = __map_domain_page(page);
         hypercall_page_initialise(d, hypercall_page);
         unmap_domain_page(hypercall_page);
-- 
1.7.10.4

Re: [PATCH v2] x86/domctl: Improve XEN_DOMCTL_hypercall_init error handling

[Jan Beulich]

>>> On 12.03.15 at 12:03, <andrew.cooper3@citrix.com> wrote:
> EACCES cannot be distinguished against an incorrect DOMCTL_INTERFACE_VERSION,
> and will cause an incorrect "need to rebuild the user-space tool set?" 
> message
> from libxc.
> 
> On the libxc side, put the useful piece of information in the error message,
> rathe than the -1 from do_domctl().
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH v2] x86/domctl: Improve XEN_DOMCTL_hypercall_init error handling

[Wei Liu]

On Thu, Mar 12, 2015 at 11:03:22AM +0000, Andrew Cooper wrote:
> EACCES cannot be distinguished against an incorrect DOMCTL_INTERFACE_VERSION,
> and will cause an incorrect "need to rebuild the user-space tool set?" message
> from libxc.
> 
> On the libxc side, put the useful piece of information in the error message,
> rathe than the -1 from do_domctl().
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> 

Acked-by: Wei Liu <wei.liu2@citrix.com>