Re: [Xen-devel] [PATCH for-4.7 3/4] tools/xsplice: fix mixing system errno values with Xen ones.

2016-05-02 Thread Wei Liu
On Fri, Apr 29, 2016 at 04:21:19PM +0200, Roger Pau Monne wrote:
> Avoid using system errno values when comparing with Xen errno values.
> 
> Signed-off-by: Roger Pau Monné 
> ---
> Cc: Konrad Rzeszutek Wilk 
> Cc: Ross Lagerwall 
> Cc: Ian Jackson 
> Cc: Wei Liu 
> ---
> Using errno values inside of hypercall structs is not right IMHO, but there
> are already several occurrences of this. Although I'm adding the correct XEN_
> prefixes here, it's very likely that new additions/modifications to this
> file will not take this into account, breaking it for OSes != Linux.

While the discussion of errno is on-going, I think we should accept this
patch for the time being.

Wei.

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


Re: [Xen-devel] [PATCH for-4.7 3/4] tools/xsplice: fix mixing system errno values with Xen ones.

2016-04-29 Thread Wei Liu
On Fri, Apr 29, 2016 at 05:12:51PM +0200, Roger Pau Monne wrote:
> On Fri, Apr 29, 2016 at 04:02:33PM +0100, Wei Liu wrote:
> > On Fri, Apr 29, 2016 at 04:21:19PM +0200, Roger Pau Monne wrote:
> > > Avoid using system errno values when comparing with Xen errno values.
> > > 
> > > Signed-off-by: Roger Pau Monné 
> > > ---
> > > Cc: Konrad Rzeszutek Wilk 
> > > Cc: Ross Lagerwall 
> > > Cc: Ian Jackson 
> > > Cc: Wei Liu 
> > > ---
> > > Using errno values inside of hypercall structs is not right IMHO, but 
> > > there
> > > are already several occurrences of this. Although I'm adding the correct 
> > > XEN_
> > > prefixes here, it's very likely that new additions/modifications to this
> > > file will not take this into account, breaking it for OSes != Linux.
> > 
> > This seems to be a rather thorny issue.
> > 
> > I have a gut feeling that returning XEN_ errno to userspace program is
> > layering violation. They should always be translated to OS level errno
> > by privcmd driver.
> 
> Yes, the error value returned from the hypercall executed is indeed 
> translated into the native OS error space. The problem here is that those 
> error codes are returned _inside_ of the specific hypercall struct, which 
> sadly privcmd doesn't know anything about.
> 
> And of course teaching privcmd about every possible hypercall struct is 
> simply impossible, since some of them are not stable (eg: domctls)
>  
> > Aren't FreeBSD and NetBSD already doing that?
> 
> As said above, this is only done for direct return codes, everything inside 
> of the struct passed to the hypercall is returned as-is.
> 
> This is a complete mess, and TBH, I don't have a clever idea about how to 
> solve it.
> 

Me neither. Maybe a new thread should be started to discuss this.

Wei.

> Roger.

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


Re: [Xen-devel] [PATCH for-4.7 3/4] tools/xsplice: fix mixing system errno values with Xen ones.

2016-04-29 Thread Wei Liu
On Fri, Apr 29, 2016 at 04:11:47PM +0100, Andrew Cooper wrote:
> On 29/04/16 16:02, Wei Liu wrote:
> > On Fri, Apr 29, 2016 at 04:21:19PM +0200, Roger Pau Monne wrote:
> >> Avoid using system errno values when comparing with Xen errno values.
> >>
> >> Signed-off-by: Roger Pau Monné 
> >> ---
> >> Cc: Konrad Rzeszutek Wilk 
> >> Cc: Ross Lagerwall 
> >> Cc: Ian Jackson 
> >> Cc: Wei Liu 
> >> ---
> >> Using errno values inside of hypercall structs is not right IMHO, but there
> >> are already several occurrences of this. Although I'm adding the correct 
> >> XEN_
> >> prefixes here, it's very likely that new additions/modifications to this
> >> file will not take this into account, breaking it for OSes != Linux.
> > This seems to be a rather thorny issue.
> >
> > I have a gut feeling that returning XEN_ errno to userspace program is
> > layering violation. They should always be translated to OS level errno
> > by privcmd driver.
> >
> > Aren't FreeBSD and NetBSD already doing that?
> 
> It is not practical for the privcmd driver to do this translation, as
> most hypercalls through the privcmd driver are toolstack calls, and have
> an unstable layout.
> 

If what you mean is what Roger said in his other reply then I agree
translating in privcmd is not doable.

> Another thorny issue is that the ioctl() call for privcmd returns errors
> via errno, which may be in system errno space, or xen errno space
> depending on the source of the errno.
> 
> This is very large swamp in desperate need of some attention.
> 

Indeed.

Wei.

> ~Andrew

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


Re: [Xen-devel] [PATCH for-4.7 3/4] tools/xsplice: fix mixing system errno values with Xen ones.

2016-04-29 Thread Roger Pau Monne
On Fri, Apr 29, 2016 at 04:02:33PM +0100, Wei Liu wrote:
> On Fri, Apr 29, 2016 at 04:21:19PM +0200, Roger Pau Monne wrote:
> > Avoid using system errno values when comparing with Xen errno values.
> > 
> > Signed-off-by: Roger Pau Monné 
> > ---
> > Cc: Konrad Rzeszutek Wilk 
> > Cc: Ross Lagerwall 
> > Cc: Ian Jackson 
> > Cc: Wei Liu 
> > ---
> > Using errno values inside of hypercall structs is not right IMHO, but there
> > are already several occurrences of this. Although I'm adding the correct 
> > XEN_
> > prefixes here, it's very likely that new additions/modifications to this
> > file will not take this into account, breaking it for OSes != Linux.
> 
> This seems to be a rather thorny issue.
> 
> I have a gut feeling that returning XEN_ errno to userspace program is
> layering violation. They should always be translated to OS level errno
> by privcmd driver.

Yes, the error value returned from the hypercall executed is indeed 
translated into the native OS error space. The problem here is that those 
error codes are returned _inside_ of the specific hypercall struct, which 
sadly privcmd doesn't know anything about.

And of course teaching privcmd about every possible hypercall struct is 
simply impossible, since some of them are not stable (eg: domctls)
 
> Aren't FreeBSD and NetBSD already doing that?

As said above, this is only done for direct return codes, everything inside 
of the struct passed to the hypercall is returned as-is.

This is a complete mess, and TBH, I don't have a clever idea about how to 
solve it.

Roger.

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


Re: [Xen-devel] [PATCH for-4.7 3/4] tools/xsplice: fix mixing system errno values with Xen ones.

2016-04-29 Thread Andrew Cooper
On 29/04/16 16:02, Wei Liu wrote:
> On Fri, Apr 29, 2016 at 04:21:19PM +0200, Roger Pau Monne wrote:
>> Avoid using system errno values when comparing with Xen errno values.
>>
>> Signed-off-by: Roger Pau Monné 
>> ---
>> Cc: Konrad Rzeszutek Wilk 
>> Cc: Ross Lagerwall 
>> Cc: Ian Jackson 
>> Cc: Wei Liu 
>> ---
>> Using errno values inside of hypercall structs is not right IMHO, but there
>> are already several occurrences of this. Although I'm adding the correct XEN_
>> prefixes here, it's very likely that new additions/modifications to this
>> file will not take this into account, breaking it for OSes != Linux.
> This seems to be a rather thorny issue.
>
> I have a gut feeling that returning XEN_ errno to userspace program is
> layering violation. They should always be translated to OS level errno
> by privcmd driver.
>
> Aren't FreeBSD and NetBSD already doing that?

It is not practical for the privcmd driver to do this translation, as
most hypercalls through the privcmd driver are toolstack calls, and have
an unstable layout.

Another thorny issue is that the ioctl() call for privcmd returns errors
via errno, which may be in system errno space, or xen errno space
depending on the source of the errno.

This is very large swamp in desperate need of some attention.

~Andrew

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


Re: [Xen-devel] [PATCH for-4.7 3/4] tools/xsplice: fix mixing system errno values with Xen ones.

2016-04-29 Thread Wei Liu
On Fri, Apr 29, 2016 at 04:21:19PM +0200, Roger Pau Monne wrote:
> Avoid using system errno values when comparing with Xen errno values.
> 
> Signed-off-by: Roger Pau Monné 
> ---
> Cc: Konrad Rzeszutek Wilk 
> Cc: Ross Lagerwall 
> Cc: Ian Jackson 
> Cc: Wei Liu 
> ---
> Using errno values inside of hypercall structs is not right IMHO, but there
> are already several occurrences of this. Although I'm adding the correct XEN_
> prefixes here, it's very likely that new additions/modifications to this
> file will not take this into account, breaking it for OSes != Linux.

This seems to be a rather thorny issue.

I have a gut feeling that returning XEN_ errno to userspace program is
layering violation. They should always be translated to OS level errno
by privcmd driver.

Aren't FreeBSD and NetBSD already doing that?

Wei.

> ---
>  tools/misc/xen-xsplice.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/misc/xen-xsplice.c b/tools/misc/xen-xsplice.c
> index 695be7f..b31115c 100644
> --- a/tools/misc/xen-xsplice.c
> +++ b/tools/misc/xen-xsplice.c
> @@ -13,6 +13,8 @@
>  #include 
>  #include 
>  
> +#include 
> +
>  static xc_interface *xch;
>  
>  void show_help(void)
> @@ -233,7 +235,7 @@ struct {
>  .function = xc_xsplice_revert,
>  },
>  {   .allow = XSPLICE_STATE_CHECKED,
> -.expected = -ENOENT,
> +.expected = -XEN_ENOENT,
>  .name = "unload",
>  .function = xc_xsplice_unload,
>  },
> @@ -276,7 +278,7 @@ int action_func(int argc, char *argv[], unsigned int idx)
>  name, errno, strerror(errno));
>  return -1;
>  }
> -if ( status.rc == -EAGAIN )
> +if ( status.rc == -XEN_EAGAIN )
>  {
>  fprintf(stderr, "%s failed. Operation already in progress\n", name);
>  return -1;
> @@ -319,7 +321,7 @@ int action_func(int argc, char *argv[], unsigned int idx)
>  
>  if ( status.state != original_state )
>  break;
> -if ( status.rc && status.rc != -EAGAIN )
> +if ( status.rc && status.rc != -XEN_EAGAIN )
>  {
>  rc = status.rc;
>  break;
> -- 
> 2.6.4 (Apple Git-63)
> 

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


[Xen-devel] [PATCH for-4.7 3/4] tools/xsplice: fix mixing system errno values with Xen ones.

2016-04-29 Thread Roger Pau Monne
Avoid using system errno values when comparing with Xen errno values.

Signed-off-by: Roger Pau Monné 
---
Cc: Konrad Rzeszutek Wilk 
Cc: Ross Lagerwall 
Cc: Ian Jackson 
Cc: Wei Liu 
---
Using errno values inside of hypercall structs is not right IMHO, but there
are already several occurrences of this. Although I'm adding the correct XEN_
prefixes here, it's very likely that new additions/modifications to this
file will not take this into account, breaking it for OSes != Linux.
---
 tools/misc/xen-xsplice.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/misc/xen-xsplice.c b/tools/misc/xen-xsplice.c
index 695be7f..b31115c 100644
--- a/tools/misc/xen-xsplice.c
+++ b/tools/misc/xen-xsplice.c
@@ -13,6 +13,8 @@
 #include 
 #include 
 
+#include 
+
 static xc_interface *xch;
 
 void show_help(void)
@@ -233,7 +235,7 @@ struct {
 .function = xc_xsplice_revert,
 },
 {   .allow = XSPLICE_STATE_CHECKED,
-.expected = -ENOENT,
+.expected = -XEN_ENOENT,
 .name = "unload",
 .function = xc_xsplice_unload,
 },
@@ -276,7 +278,7 @@ int action_func(int argc, char *argv[], unsigned int idx)
 name, errno, strerror(errno));
 return -1;
 }
-if ( status.rc == -EAGAIN )
+if ( status.rc == -XEN_EAGAIN )
 {
 fprintf(stderr, "%s failed. Operation already in progress\n", name);
 return -1;
@@ -319,7 +321,7 @@ int action_func(int argc, char *argv[], unsigned int idx)
 
 if ( status.state != original_state )
 break;
-if ( status.rc && status.rc != -EAGAIN )
+if ( status.rc && status.rc != -XEN_EAGAIN )
 {
 rc = status.rc;
 break;
-- 
2.6.4 (Apple Git-63)


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