From: Roger Pau Monne <roger.pau@citrix.com>
To: Wei Liu <wei.liu2@citrix.com>
Cc: andrew.cooper3@citrix.com,
Ian Jackson <ian.jackson@eu.citrix.com>,
Ross Lagerwall <ross.lagerwall@citrix.com>,
JBeulich@suse.com, xen-devel@lists.xenproject.org
Subject: Returning errno values inside of hypercall structs (was: Re: [PATCH for-4.7 3/4] tools/xsplice: fix mixing system)
Date: Fri, 29 Apr 2016 18:16:19 +0200 [thread overview]
Message-ID: <20160429161619.hy57yirgq7xzwdhd@mac> (raw)
On Fri, Apr 29, 2016 at 04:30:16PM +0100, Wei Liu wrote:
> 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:
> > > 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.
So here we are.
In order to put everyone into context: the issue here is that some
hypercalls (those that batch several operations) return an array of error
codes inside of the hypercall structure. This array of error codes is not
standardized, so the privcmd driver doesn't know anything about it, and thus
cannot translate it into the native OS error space.
It has also been suggested that the privcmd driver simply doesn't translate
error codes at all, and then let the applications figure out if the error
code comes from Xen or from the OS. IMHO, this is impossible to achieve,
because the ioctl syscall can return an error code that's been forwarded
by Xen or a native one, and the application has no way of knowing where is
it coming from.
I've identified at least the following hypercall structs that store XEN_*
error codes inside:
- xen_add_to_physmap_batch
- xen_xsplice_status
TBH, it's quite hard to spot them, so I might have missed some.
xen_add_to_physmap_batch is part of the public ABI, and cannot be changed.
On the bright side, xen_add_to_physmap_batch is implemented as a different
ioctl in privcmd usually (in order to map memory from other domains), so the
error translation should be handled correctly.
Then the xsplice struct that uses XEN_* values is:
struct xen_xsplice_status {
#define XSPLICE_STATE_CHECKED 1
#define XSPLICE_STATE_APPLIED 2
uint32_t state; /* OUT: XSPLICE_STATE_*. */
int32_t rc; /* OUT: 0 if no error, otherwise -XEN_EXX. */
};
Which is in turn used by:
struct xen_sysctl_xsplice_list {
uint32_t version; /* OUT: Hypervisor stamps value.
If varies between calls, we are
* getting stale data. */
uint32_t idx; /* IN: Index into hypervisor list. */
uint32_t nr; /* IN: How many status, name, and len
should fill out. Can be zero to get
amount of payloads and version.
OUT: How many payloads left. */
uint32_t pad; /* IN: Must be zero. */
XEN_GUEST_HANDLE_64(xen_xsplice_status_t) status; /* OUT. Must have enough
space allocate for nr of them. */
XEN_GUEST_HANDLE_64(char) name; /* OUT: Array of names. Each member
MUST XEN_XSPLICE_NAME_SIZE in size.
Must have nr of them. */
XEN_GUEST_HANDLE_64(uint32) len; /* OUT: Array of lengths of name's.
Must have nr of them. */
};
IMHO, the best way to solve this is to define a set of XSPLICE_ERROR_* that
covers the error codes returned by xsplice, and use that instead of XEN_*
errno values. This would make it much more easier to avoid mistakes when
coding the toolstack part of xsplice.
Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
next reply other threads:[~2016-04-29 16:16 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-29 16:16 Roger Pau Monne [this message]
2016-04-29 16:22 ` Returning errno values inside of hypercall structs (was: Re: [PATCH for-4.7 3/4] tools/xsplice: fix mixing system) Konrad Rzeszutek Wilk
2016-04-29 16:31 ` Roger Pau Monne
2016-04-29 16:59 ` Konrad Rzeszutek Wilk
2016-04-29 17:07 ` Roger Pau Monne
2016-04-29 17:25 ` Konrad Rzeszutek Wilk
2016-04-29 16:26 ` Jan Beulich
2016-05-03 10:49 ` David Vrabel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160429161619.hy57yirgq7xzwdhd@mac \
--to=roger.pau@citrix.com \
--cc=JBeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=ian.jackson@eu.citrix.com \
--cc=ross.lagerwall@citrix.com \
--cc=wei.liu2@citrix.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).