[PATCH v2 for-4.6 0/3] Get rid of non-POSIX error codes

[PATCH v2 for-4.6 0/3] Get rid of non-POSIX error codes

[Roger Pau Monne]

This patch series gets rid of non-POSIX error codes in the hypervisor and 
the toolstack. This is needed for OS compatibility.

I think the patch series is fairly small and non-intrusive, hence the 
for-4.6 tag.

Thanks, Roger.

[PATCH v2 for-4.6 1/3] xen/x86/libxl: replace non-POSIX error codes used by PSR code

[Roger Pau Monne]

PSR was using EBADSLT and EUSERS which are not POSIX error codes, replace
them with ENOTSOCK and EOVERFLOW respectively.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes since v1:
 - Use ENOTSOCK to replace EBADSLT instead of EINVAL.
---
 tools/libxl/libxl_psr.c    | 6 +++---
 xen/arch/x86/psr.c         | 8 ++++----
 xen/include/public/errno.h | 1 +
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 2a07777..3378239 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -31,7 +31,7 @@ static void libxl__psr_log_err_msg(libxl__gc *gc, int err)
     case ESRCH:
         msg = "invalid domain ID";
         break;
-    case EBADSLT:
+    case ENOTSOCK:
         msg = "socket is not supported";
         break;
     case EFAULT:
@@ -59,7 +59,7 @@ static void libxl__psr_cmt_log_err_msg(libxl__gc *gc, int err)
     case ENOENT:
         msg = "CMT is not attached to this domain";
         break;
-    case EUSERS:
+    case EOVERFLOW:
         msg = "no free RMID available";
         break;
     default:
@@ -81,7 +81,7 @@ static void libxl__psr_cat_log_err_msg(libxl__gc *gc, int err)
     case ENOENT:
         msg = "CAT is not enabled on the socket";
         break;
-    case EUSERS:
+    case EOVERFLOW:
         msg = "no free COS available";
         break;
     case EEXIST:
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 861683f..19f714b 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -176,7 +176,7 @@ int psr_alloc_rmid(struct domain *d)
     if ( rmid > psr_cmt->rmid_max )
     {
         d->arch.psr_rmid = 0;
-        return -EUSERS;
+        return -EOVERFLOW;
     }
 
     d->arch.psr_rmid = rmid;
@@ -251,7 +251,7 @@ static struct psr_cat_socket_info *get_cat_socket_info(unsigned int socket)
         return ERR_PTR(-ENODEV);
 
     if ( socket >= nr_sockets )
-        return ERR_PTR(-EBADSLT);
+        return ERR_PTR(-ENOTSOCK);
 
     if ( !test_bit(socket, cat_socket_enable) )
         return ERR_PTR(-ENOENT);
@@ -332,7 +332,7 @@ static int write_l3_cbm(unsigned int socket, unsigned int cos, uint64_t cbm)
         unsigned int cpu = get_socket_cpu(socket);
 
         if ( cpu >= nr_cpu_ids )
-            return -EBADSLT;
+            return -ENOTSOCK;
         on_selected_cpus(cpumask_of(cpu), do_write_l3_cbm, &info, 1);
     }
 
@@ -381,7 +381,7 @@ int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
     if ( !found )
     {
         spin_unlock(&info->cbm_lock);
-        return -EUSERS;
+        return -EOVERFLOW;
     }
 
     cos = found - map;
diff --git a/xen/include/public/errno.h b/xen/include/public/errno.h
index 9a7e411..50adcb9 100644
--- a/xen/include/public/errno.h
+++ b/xen/include/public/errno.h
@@ -65,6 +65,7 @@ XEN_ERRNO(EILSEQ,	84)	/* Illegal byte sequence */
 XEN_ERRNO(ERESTART,	85)	/* Interrupted system call should be restarted */
 #endif
 XEN_ERRNO(EUSERS,	87)	/* Too many users */
+XEN_ERRNO(ENOTSOCK,	88)	/* Socket operation on non-socket */
 XEN_ERRNO(EOPNOTSUPP,	95)	/* Operation not supported on transport endpoint */
 XEN_ERRNO(EADDRINUSE,	98)	/* Address already in use */
 XEN_ERRNO(EADDRNOTAVAIL, 99)	/* Cannot assign requested address */
-- 
1.9.5 (Apple Git-50.3)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2 for-4.6 1/3] xen/x86/libxl: replace non-POSIX error codes used by PSR code

[Wei Liu]

On Wed, Jul 22, 2015 at 01:07:50PM +0200, Roger Pau Monne wrote:
> PSR was using EBADSLT and EUSERS which are not POSIX error codes, replace
> them with ENOTSOCK and EOVERFLOW respectively.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>

With my tools maintainer hat on:

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

[PATCH v2 for-4.6 2/3] xen: replace non-POSIX error codes

[Roger Pau Monne]

Some DOMCTLs returned non-POSIX error codes, replace them with POSIX
compilant values instead. EBADRQC and EBADSLT are replaced by EDOM, while
EUSERS is replaced with EOVERFLOW.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes since v1:
 - Use EDOM instead of EINVAL.
---
Nothing in libxc or libxl seems to check for those specific error codes, so
I guess it's fine to replace them with whatever we want.
---
 xen/arch/x86/mm/paging.c | 2 +-
 xen/common/domain.c      | 4 ++--
 xen/common/hvm/save.c    | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
index 7089155..9bd54a8 100644
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -766,7 +766,7 @@ long paging_domctl_continuation(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
 
     if ( op.interface_version != XEN_DOMCTL_INTERFACE_VERSION ||
          op.cmd != XEN_DOMCTL_shadow_op )
-        return -EBADRQC;
+        return -EDOM;
 
     d = rcu_lock_domain_by_id(op.domain);
     if ( d == NULL )
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 8efef5c..791166b 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -900,7 +900,7 @@ int vcpu_pause_by_systemcontroller(struct vcpu *v)
         new = old + 1;
 
         if ( new > 255 )
-            return -EUSERS;
+            return -EOVERFLOW;
 
         prev = cmpxchg(&v->controller_pause_count, old, new);
     } while ( prev != old );
@@ -980,7 +980,7 @@ int __domain_pause_by_systemcontroller(struct domain *d,
          * toolstack overflowing d->pause_count with many repeated hypercalls.
          */
         if ( new > 255 )
-            return -EUSERS;
+            return -EOVERFLOW;
 
         prev = cmpxchg(&d->controller_pause_count, old, new);
     } while ( prev != old );
diff --git a/xen/common/hvm/save.c b/xen/common/hvm/save.c
index da6e668..56589fa 100644
--- a/xen/common/hvm/save.c
+++ b/xen/common/hvm/save.c
@@ -114,7 +114,7 @@ int hvm_save_one(struct domain *d, uint16_t typecode, uint16_t instance,
         uint32_t off;
         const struct hvm_save_descriptor *desc;
 
-        rv = -EBADSLT;
+        rv = -EDOM;
         for ( off = 0; off < (ctxt.cur - sizeof(*desc)); off += desc->length )
         {
             desc = (void *)(ctxt.data + off);
-- 
1.9.5 (Apple Git-50.3)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2 for-4.6 2/3] xen: replace non-POSIX error codes

[George Dunlap]

On 07/22/2015 12:07 PM, Roger Pau Monne wrote:
> Some DOMCTLs returned non-POSIX error codes, replace them with POSIX
> compilant values instead. EBADRQC and EBADSLT are replaced by EDOM, while
> EUSERS is replaced with EOVERFLOW.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> Changes since v1:
>  - Use EDOM instead of EINVAL.
> ---
> Nothing in libxc or libxl seems to check for those specific error codes, so
> I guess it's fine to replace them with whatever we want.
> ---
>  xen/arch/x86/mm/paging.c | 2 +-
>  xen/common/domain.c      | 4 ++--
>  xen/common/hvm/save.c    | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
> index 7089155..9bd54a8 100644
> --- a/xen/arch/x86/mm/paging.c
> +++ b/xen/arch/x86/mm/paging.c
> @@ -766,7 +766,7 @@ long paging_domctl_continuation(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>  
>      if ( op.interface_version != XEN_DOMCTL_INTERFACE_VERSION ||
>           op.cmd != XEN_DOMCTL_shadow_op )
> -        return -EBADRQC;
> +        return -EDOM;

mm bit:

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

Since this is internal, this isn't even a bike shed -- it's debating the
color of the insulation on the pipes in the basement.  Feel free to
retain this Ack even if you end up changing this particular errno to
something else again.

 -George


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2 for-4.6 2/3] xen: replace non-POSIX error codes

[Jan Beulich]

>>> On 22.07.15 at 13:07, <roger.pau@citrix.com> wrote:
> --- a/xen/common/hvm/save.c
> +++ b/xen/common/hvm/save.c
> @@ -114,7 +114,7 @@ int hvm_save_one(struct domain *d, uint16_t typecode, uint16_t instance,
>          uint32_t off;
>          const struct hvm_save_descriptor *desc;
>  
> -        rv = -EBADSLT;
> +        rv = -EDOM;
>          for ( off = 0; off < (ctxt.cur - sizeof(*desc)); off += desc->length )
>          {
>              desc = (void *)(ctxt.data + off);

In what sense do you view EDOM applicable here? It looks pretty
strange to me (also the one further up, but as George says the
precise value there doesn't really matter all that much). ENXIO or
ENOENT would seem more suitable here (and for the one above I'd
rather pick e.g. EACCES, EOPNOTSUPP, or even ENOSYS). I'd be
fine adjusting these on commit, i.e. no need to re-submit (but
please let me know if you have a preference for any of the
suggestions).

In any event, consider the whole series
Acked-by: Jan Beulich <jbeulich@suse.com>
(or the other two, and with the changes made also this one,
even Reviewed-by).

Jan

[PATCH v2 for-4.6 3/3] xen: remove non-POSIX error codes

[Roger Pau Monne]

Xen was using some non-POSIX error codes that are removed in this patch. For
future reference, the list of POSIX error codes has been obtained from:

http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html

The error codes already present and defined as optional (XSR), have been
left in place.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Tim Deegan <tim@xen.org>
---
 xen/include/public/errno.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/xen/include/public/errno.h b/xen/include/public/errno.h
index 50adcb9..db9d064 100644
--- a/xen/include/public/errno.h
+++ b/xen/include/public/errno.h
@@ -54,8 +54,6 @@ XEN_ERRNO(EDEADLK,	35)	/* Resource deadlock would occur */
 XEN_ERRNO(ENAMETOOLONG,	36)	/* File name too long */
 XEN_ERRNO(ENOLCK,	37)	/* No record locks available */
 XEN_ERRNO(ENOSYS,	38)	/* Function not implemented */
-XEN_ERRNO(EBADRQC,	56)	/* Invalid request code */
-XEN_ERRNO(EBADSLT,	57)	/* Invalid slot */
 XEN_ERRNO(ENODATA,	61)	/* No data available */
 XEN_ERRNO(ETIME,	62)	/* Timer expired */
 XEN_ERRNO(EBADMSG,	74)	/* Not a data message */
@@ -64,7 +62,6 @@ XEN_ERRNO(EILSEQ,	84)	/* Illegal byte sequence */
 #ifdef __XEN__ /* Internal only, should never be exposed to the guest. */
 XEN_ERRNO(ERESTART,	85)	/* Interrupted system call should be restarted */
 #endif
-XEN_ERRNO(EUSERS,	87)	/* Too many users */
 XEN_ERRNO(ENOTSOCK,	88)	/* Socket operation on non-socket */
 XEN_ERRNO(EOPNOTSUPP,	95)	/* Operation not supported on transport endpoint */
 XEN_ERRNO(EADDRINUSE,	98)	/* Address already in use */
@@ -72,8 +69,6 @@ XEN_ERRNO(EADDRNOTAVAIL, 99)	/* Cannot assign requested address */
 XEN_ERRNO(ENOBUFS,	105)	/* No buffer space available */
 XEN_ERRNO(EISCONN,	106)	/* Transport endpoint is already connected */
 XEN_ERRNO(ENOTCONN,	107)	/* Transport endpoint is not connected */
-XEN_ERRNO(ESHUTDOWN,	108)	/* Cannot send after transport endpoint shutdown */
-XEN_ERRNO(ETOOMANYREFS,	109)	/* Too many references: cannot splice */
 XEN_ERRNO(ETIMEDOUT,	110)	/* Connection timed out */
 
 #undef XEN_ERRNO
-- 
1.9.5 (Apple Git-50.3)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2 for-4.6 3/3] xen: remove non-POSIX error codes

[Ian Campbell]

On Wed, 2015-07-22 at 13:07 +0200, Roger Pau Monne wrote:
> Xen was using some non-POSIX error codes that are removed in this 
> patch. For
> future reference, the list of POSIX error codes has been obtained 
> from:
> 
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html
> 
> The error codes already present and defined as optional (XSR), have 
> been
> left in place.

I think it would be worth adding a comment to the header including a
link to this and noting that although some optional (XSR) values are
present (having been grandfathered in) no _new_ ones should be added.

But that could be done in a followup, since Jan already acked this.

Ian.

Re: [PATCH v2 for-4.6 3/3] xen: remove non-POSIX error codes

[Jan Beulich]

>>> On 22.07.15 at 16:16, <ian.campbell@citrix.com> wrote:
> On Wed, 2015-07-22 at 13:07 +0200, Roger Pau Monne wrote:
>> Xen was using some non-POSIX error codes that are removed in this 
>> patch. For
>> future reference, the list of POSIX error codes has been obtained 
>> from:
>> 
>> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html 
>> 
>> The error codes already present and defined as optional (XSR), have 
>> been
>> left in place.
> 
> I think it would be worth adding a comment to the header including a
> link to this and noting that although some optional (XSR) values are
> present (having been grandfathered in) no _new_ ones should be added.

While committing the patch I extended a comment there along
those lines.

Jan

Re: [PATCH v2 for-4.6 0/3] Get rid of non-POSIX error codes

[Jan Beulich]

>>> On 22.07.15 at 13:07, <roger.pau@citrix.com> wrote:
> This patch series gets rid of non-POSIX error codes in the hypervisor and 
> the toolstack. This is needed for OS compatibility.
> 
> I think the patch series is fairly small and non-intrusive, hence the 
> for-4.6 tag.

In which case you should have Cc-ed Wei, so he can ack (or nak) this.
I certainly support this going in now, as otherwise we'd ship with non-
POSIX compliant public errno.h.

Jan

Re: [PATCH v2 for-4.6 0/3] Get rid of non-POSIX error codes

[Wei Liu]

On Wed, Jul 22, 2015 at 06:02:28AM -0600, Jan Beulich wrote:
> >>> On 22.07.15 at 13:07, <roger.pau@citrix.com> wrote:
> > This patch series gets rid of non-POSIX error codes in the hypervisor and 
> > the toolstack. This is needed for OS compatibility.
> > 
> > I think the patch series is fairly small and non-intrusive, hence the 
> > for-4.6 tag.
> 
> In which case you should have Cc-ed Wei, so he can ack (or nak) this.
> I certainly support this going in now, as otherwise we'd ship with non-
> POSIX compliant public errno.h.
> 

I have no preference on which specific error numbers get used.  I'm fine
with this series going in principle. 

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

> Jan