[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 4/5] 9pfs: fix wrong errno being sent to Linux client on macO
From: |
Christian Schoenebeck |
Subject: |
Re: [PATCH 4/5] 9pfs: fix wrong errno being sent to Linux client on macOS host |
Date: |
Thu, 21 Apr 2022 13:13:08 +0200 |
On Donnerstag, 21. April 2022 12:48:35 CEST Greg Kurz wrote:
> On Tue, 19 Apr 2022 13:41:59 +0200
>
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > Linux and macOS only share some errno definitions with equal macro
> > name and value. In fact most mappings for errno are completely
> > different on the two systems.
> >
> > This patch converts some important errno values from macOS host to
> > corresponding Linux errno values before eventually sending such error
> > codes along with Tlerror replies (if 9p2000.L is used that is), which
> > fixes a bunch of misbehaviours when running a Linux client on macOS
> > host.
>
> This even fixes an actual protocol violation :
>
> lerror -- return error code
>
> size[4] Rlerror tag[2] ecode[4]
>
> lerror replaces the reply message used in a successful call. ecode is a
> numerical Linux errno.
> ^^^^^^^^^^^^^^^^^^^^^
>
> Taken from
> https://github.com/chaos/diod/wiki/protocol#lerror----return-error-code
Again, something to add to the commit log?
> > For instance this patch fixes:
> > mount -t 9p -o posixacl ...
> >
> > on Linux guest if security_mode=mapped was used for 9p server, which
> > refused to mount successfully, because macOS returned ENOATTR==93
> > when client tried to retrieve POSIX ACL xattrs, because errno 93
> > is defined as EPROTONOSUPPORT==93 on Linux, so Linux client believed
> > that xattrs were not supported by filesystem on host in general.
> >
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > ---
> >
> > hw/9pfs/9p.c | 27 +++++++++++++++++++++++++++
> > 1 file changed, 27 insertions(+)
> >
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index d953035e1c..becc41cbfd 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -57,6 +57,31 @@ enum {
> >
> > P9ARRAY_DEFINE_TYPE(V9fsPath, v9fs_path_free);
> >
> > +/* Translates errno from host -> Linux if needed */
> > +static int errno_to_dotl(int err) {
> > +#if defined(CONFIG_LINUX)
> > + /* nothing to translate (Linux -> Linux) */
> > +#elif defined(CONFIG_DARWIN)
> > + /* translation mandatory for macOS hosts */
> > + if (err == ENAMETOOLONG) {
> > + err = 36; /* ==ENAMETOOLONG on Linux */
> > + } else if (err == ENOTEMPTY) {
> > + err = 39; /* ==ENOTEMPTY on Linux */
> > + } else if (err == ELOOP) {
> > + err = 40; /* ==ELOOP on Linux */
> > + } else if (err == ENOATTR) {
> > + err = 61; /* ==ENODATA on Linux */
> > + } else if (err == ENOTSUP) {
> > + err = 95; /* ==EOPNOTSUPP on Linux */
> > + } else if (err == EOPNOTSUPP) {
> > + err = 95; /* ==EOPNOTSUPP on Linux */
> > + }
>
> I'm assuming you have audited all errnos, right ? Just to be sure
> that this won't bite anymore.
Depends on what you mean with "all". Like I wrote in the commit log, for now I
translated in this patch those errnos that I identified as important, and
those are audited by me of course (checked the man pages for what errors are
expected as result from calls on Linux vs. macOS side and looked up numeric
values from header files on both sides, tests).
However if you rather mean really all errnos that were ever defined on Linux
and macOS, then the answer is no. That would probably quite some work, and I'm
not sure if you could just try to translate them dry, i.e. by just looking at
the headers or something.
So yes, your concern is justified. The question is, should this all be
translated right now already, or would it be OK to address this minimum set of
errno translation for now (especially for qemu-stable) and then later address
the rest?
On the long term you would probably import the Linux errno header file into
the code base, then prefix the individual macros with something like
DOTL_ENODATA, etc. and then use those macros for translating the errnos
instead of using literal numerics, maybe using GCC's designated array
initializers then.
> > +#else
> > +#error Missing errno translation to Linux for this host system
> > +#endif
> > + return err;
> > +}
> > +
>
> As with the other patch, I'd rather move this magic to 9p-util.h .
Makes sense. There is indeed already too much utility code piled up in 9p.c.
> > static ssize_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt,
> > ...) {
> >
> > ssize_t ret;
> >
> > @@ -1054,6 +1079,8 @@ static void coroutine_fn pdu_complete(V9fsPDU *pdu,
> > ssize_t len)>
> > }
> > len += ret;
> > id = P9_RERROR;
> >
> > + } else {
> > + err = errno_to_dotl(err);
> >
> > }
> >
> > ret = pdu_marshal(pdu, len, "d", err);
- Re: [PATCH 2/5] 9pfs: fix qemu_mknodat(S_IFSOCK) on macOS, (continued)
Re: [PATCH 2/5] 9pfs: fix qemu_mknodat(S_IFSOCK) on macOS, Greg Kurz, 2022/04/20
[PATCH 3/5] 9pfs: fix wrong encoding of rdev field in Rgetattr on macOS, Christian Schoenebeck, 2022/04/19
[PATCH 4/5] 9pfs: fix wrong errno being sent to Linux client on macOS host, Christian Schoenebeck, 2022/04/19
[PATCH 5/5] 9pfs: fix removing non-existent POSIX ACL xattr on macOS host, Christian Schoenebeck, 2022/04/19