[PATCH v5 1/6] 9pfs: fix qemu_mknodat(S_IFREG) on macOS

[PATCH v5 1/6] 9pfs: fix qemu_mknodat(S_IFREG) on macOS

[Christian Schoenebeck]

mknod() on macOS does not support creating regular files, so
divert to openat_file() if S_IFREG is passed with mode argument.

Furthermore, 'man 2 mknodat' on Linux says: "Zero file type is
equivalent to type S_IFREG".

Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Will Cohen <wwcohen@gmail.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/9pfs/9p-util-darwin.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
index bec0253474..e24d09763a 100644
--- a/hw/9pfs/9p-util-darwin.c
+++ b/hw/9pfs/9p-util-darwin.c
@@ -77,6 +77,15 @@ int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
 int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
 {
     int preserved_errno, err;
+
+    if (S_ISREG(mode) || !(mode & S_IFMT)) {
+        int fd = openat_file(dirfd, filename, O_CREAT, mode);
+        if (fd == -1) {
+            return fd;
+        }
+        close(fd);
+        return 0;
+    }
     if (!pthread_fchdir_np) {
         error_report_once("pthread_fchdir_np() not available on this version of macOS");
         return -ENOTSUP;
-- 
2.32.0 (Apple Git-132)

[PATCH v5 2/6] 9pfs: fix qemu_mknodat(S_IFSOCK) on macOS

[Christian Schoenebeck]

mknod() on macOS does not support creating sockets, so divert to
call sequence socket(), bind() and fchmodat() respectively if S_IFSOCK
was passed with mode argument.

Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/9p-util-darwin.c | 42 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
index e24d09763a..619c403ba7 100644
--- a/hw/9pfs/9p-util-darwin.c
+++ b/hw/9pfs/9p-util-darwin.c
@@ -74,6 +74,42 @@ int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
  */
 #if defined CONFIG_PTHREAD_FCHDIR_NP
 
+static int create_socket_file_at_cwd(const char *filename, mode_t mode) {
+    int fd, err;
+    struct sockaddr_un addr = {
+        .sun_family = AF_UNIX
+    };
+
+    err = snprintf(addr.sun_path, sizeof(addr.sun_path), "./%s", filename);
+    if (err < 0 || err >= sizeof(addr.sun_path)) {
+        errno = ENAMETOOLONG;
+        return -1;
+    }
+    fd = socket(PF_UNIX, SOCK_DGRAM, 0);
+    if (fd == -1) {
+        return fd;
+    }
+    err = bind(fd, (struct sockaddr *) &addr, sizeof(addr));
+    if (err == -1) {
+        goto out;
+    }
+    /*
+     * FIXME: Should rather be using descriptor-based fchmod() on the
+     * socket file descriptor above (preferably before bind() call),
+     * instead of path-based fchmodat(), to prevent concurrent transient
+     * state issues between creating the named FIFO file at bind() and
+     * delayed adjustment of permissions at fchmodat(). However currently
+     * macOS (12.x) does not support such operations on socket file
+     * descriptors yet.
+     *
+     * Filed report with Apple: FB9997731
+     */
+    err = fchmodat(AT_FDCWD, filename, mode, AT_SYMLINK_NOFOLLOW_ANY);
+out:
+    close_preserve_errno(fd);
+    return err;
+}
+
 int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
 {
     int preserved_errno, err;
@@ -93,7 +129,11 @@ int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
     if (pthread_fchdir_np(dirfd) < 0) {
         return -1;
     }
-    err = mknod(filename, mode, dev);
+    if (S_ISSOCK(mode)) {
+        err = create_socket_file_at_cwd(filename, mode);
+    } else {
+        err = mknod(filename, mode, dev);
+    }
     preserved_errno = errno;
     /* Stop using the thread-local cwd */
     pthread_fchdir_np(-1);
-- 
2.32.0 (Apple Git-132)

[PATCH v5 3/6] 9pfs: fix wrong encoding of rdev field in Rgetattr on macOS

[Christian Schoenebeck]

The 'rdev' field in 9p reponse 'Rgetattr' is of type dev_t,
which is actually a system dependant type and therefore both the
size and encoding of dev_t differ between macOS and Linux.

So far we have sent 'rdev' to guest in host's dev_t format as-is,
which caused devices to appear with wrong device numbers on
guests running on macOS hosts, eventually leading to various
misbehaviours on guest in conjunction with device files.

This patch fixes this issue by converting the device number from
host's dev_t format to Linux dev_t format. As 9p request
'Tgettattr' is exclusive to protocol version 9p2000.L, it should
be fair to assume that 'rdev' field is assumed to be in Linux dev_t
format by client as well.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Link: https://lore.kernel.org/qemu-devel/20220421093056.5ab1e7ed@bahia/
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/9pfs/9p-util.h | 39 +++++++++++++++++++++++++++++++++++++++
 hw/9pfs/9p.c      |  2 +-
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 97e681e167..2cc9a5dbfb 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -19,6 +19,45 @@
 #define O_PATH_9P_UTIL 0
 #endif
 
+#if !defined(CONFIG_LINUX)
+
+/*
+ * Generates a Linux device number (a.k.a. dev_t) for given device major
+ * and minor numbers.
+ *
+ * To be more precise: it generates a device number in glibc's format
+ * (MMMM_Mmmm_mmmM_MMmm, 64 bits) actually, which is compatible with
+ * Linux's format (mmmM_MMmm, 32 bits), as described in <bits/sysmacros.h>.
+ */
+static inline uint64_t makedev_dotl(uint32_t dev_major, uint32_t dev_minor)
+{
+    uint64_t dev;
+
+    // from glibc sysmacros.h:
+    dev  = (((uint64_t) (dev_major & 0x00000fffu)) <<  8);
+    dev |= (((uint64_t) (dev_major & 0xfffff000u)) << 32);
+    dev |= (((uint64_t) (dev_minor & 0x000000ffu)) <<  0);
+    dev |= (((uint64_t) (dev_minor & 0xffffff00u)) << 12);
+    return dev;
+}
+
+#endif
+
+/*
+ * Converts given device number from host's device number format to Linux
+ * device number format. As both the size of type dev_t and encoding of
+ * dev_t is system dependant, we have to convert them for Linux guests if
+ * host is not running Linux.
+ */
+static inline uint64_t host_dev_to_dotl_dev(dev_t dev)
+{
+#ifdef CONFIG_LINUX
+    return dev;
+#else
+    return makedev_dotl(major(dev), minor(dev));
+#endif
+}
+
 #ifdef CONFIG_DARWIN
 #define qemu_fgetxattr(...) fgetxattr(__VA_ARGS__, 0, 0)
 #define qemu_lgetxattr(...) getxattr(__VA_ARGS__, 0, XATTR_NOFOLLOW)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 225f31fc31..4a296a0b94 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1327,7 +1327,7 @@ static int stat_to_v9stat_dotl(V9fsPDU *pdu, const struct stat *stbuf,
     v9lstat->st_nlink = stbuf->st_nlink;
     v9lstat->st_uid = stbuf->st_uid;
     v9lstat->st_gid = stbuf->st_gid;
-    v9lstat->st_rdev = stbuf->st_rdev;
+    v9lstat->st_rdev = host_dev_to_dotl_dev(stbuf->st_rdev);
     v9lstat->st_size = stbuf->st_size;
     v9lstat->st_blksize = stat_to_iounit(pdu, stbuf);
     v9lstat->st_blocks = stbuf->st_blocks;
-- 
2.32.0 (Apple Git-132)

[PATCH v5 4/6] 9pfs: fix wrong errno being sent to Linux client on macOS host

[Christian Schoenebeck]

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 'Rlerror' replies (if 9p2000.L is used that is). Not
having translated errnos before violated the 9p2000.L protocol spec,
which says:

  "
  size[4] Rlerror tag[2] ecode[4]

  ... ecode is a numerical Linux errno.
  "

  https://github.com/chaos/diod/wiki/protocol#lerror----return-error-code

This patch fixes a bunch of misbehaviours when running a Linux client
on macOS host. 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>
Link: https://lore.kernel.org/qemu-devel/20220421124835.3e664669@bahia/
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/9pfs/9p-util.h | 30 ++++++++++++++++++++++++++++++
 hw/9pfs/9p.c      |  2 ++
 2 files changed, 32 insertions(+)

diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 2cc9a5dbfb..c3526144c9 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -58,6 +58,36 @@ static inline uint64_t host_dev_to_dotl_dev(dev_t dev)
 #endif
 }
 
+/* Translates errno from host -> Linux if needed */
+static inline int errno_to_dotl(int err) {
+#if defined(CONFIG_LINUX)
+    /* nothing to translate (Linux -> Linux) */
+#elif defined(CONFIG_DARWIN)
+    /*
+     * translation mandatory for macOS hosts
+     *
+     * FIXME: Only most important errnos translated here yet, this should be
+     * extended to as many errnos being translated as possible in future.
+     */
+    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 */
+    }
+#else
+#error Missing errno translation to Linux for this host system
+#endif
+    return err;
+}
+
 #ifdef CONFIG_DARWIN
 #define qemu_fgetxattr(...) fgetxattr(__VA_ARGS__, 0, 0)
 #define qemu_lgetxattr(...) getxattr(__VA_ARGS__, 0, XATTR_NOFOLLOW)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 4a296a0b94..0cd0c14c2a 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1054,6 +1054,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);
-- 
2.32.0 (Apple Git-132)

[PATCH v5 5/6] 9pfs: fix removing non-existent POSIX ACL xattr on macOS host

[Christian Schoenebeck]

When mapped POSIX ACL is used, we are ignoring errors when trying
to remove a POSIX ACL xattr that does not exist. On Linux hosts we
would get ENODATA in such cases, on macOS hosts however we get
ENOATTR instead.

As we can be sure that ENOATTR is defined as being identical on Linux
hosts (at least by qemu/xattr.h), it is safe to fix this issue by
simply comparing against ENOATTR instead of ENODATA.

This patch fixes e.g. a command on Linux guest like:

  cp --preserve=mode old new

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Link: https://lore.kernel.org/qemu-devel/2866993.yOYK24bMf6@silver/
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/9pfs/9p-posix-acl.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/9p-posix-acl.c b/hw/9pfs/9p-posix-acl.c
index eadae270dd..4b2cb3c66c 100644
--- a/hw/9pfs/9p-posix-acl.c
+++ b/hw/9pfs/9p-posix-acl.c
@@ -65,7 +65,11 @@ static int mp_pacl_removexattr(FsContext *ctx,
     int ret;
 
     ret = local_removexattr_nofollow(ctx, path, MAP_ACL_ACCESS);
-    if (ret == -1 && errno == ENODATA) {
+    /*
+     * macOS returns ENOATTR (!=ENODATA on macOS), whereas Linux returns
+     * ENODATA (==ENOATTR on Linux), so checking for ENOATTR is fine
+     */
+    if (ret == -1 && errno == ENOATTR) {
         /*
          * We don't get ENODATA error when trying to remove a
          * posix acl that is not present. So don't throw the error
@@ -115,7 +119,11 @@ static int mp_dacl_removexattr(FsContext *ctx,
     int ret;
 
     ret = local_removexattr_nofollow(ctx, path, MAP_ACL_DEFAULT);
-    if (ret == -1 && errno == ENODATA) {
+    /*
+     * macOS returns ENOATTR (!=ENODATA on macOS), whereas Linux returns
+     * ENODATA (==ENOATTR on Linux), so checking for ENOATTR is fine
+     */
+    if (ret == -1 && errno == ENOATTR) {
         /*
          * We don't get ENODATA error when trying to remove a
          * posix acl that is not present. So don't throw the error
-- 
2.32.0 (Apple Git-132)

[PATCH v5 6/6] 9pfs: fix qemu_mknodat() to always return -1 on error on macOS host

[Christian Schoenebeck]

qemu_mknodat() is expected to behave according to its POSIX API, and
therefore should always return exactly -1 on any error, and errno
should be set for the actual error code.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/9pfs/9p-util-darwin.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
index 619c403ba7..a5f8707bb8 100644
--- a/hw/9pfs/9p-util-darwin.c
+++ b/hw/9pfs/9p-util-darwin.c
@@ -124,7 +124,8 @@ int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
     }
     if (!pthread_fchdir_np) {
         error_report_once("pthread_fchdir_np() not available on this version of macOS");
-        return -ENOTSUP;
+        errno = ENOTSUP;
+        return -1;
     }
     if (pthread_fchdir_np(dirfd) < 0) {
         return -1;
-- 
2.32.0 (Apple Git-132)

[PATCH v5 0/6] 9pfs: macOS host fixes

[Christian Schoenebeck]

A bunch of fixes for recently (in QEMU 7.0) added 9p support on macOS hosts.

Note: there are still issues to address with case-insensitive file systems
on macOS hosts. I sent a separate RFC on that icase issue:
https://lore.kernel.org/qemu-devel/1757498.AyhHxzoH2B@silver/

v4 -> v5:

  * Check return value of snprintf() instead of strlen(filename).
    [patch 2]

Christian Schoenebeck (6):
  9pfs: fix qemu_mknodat(S_IFREG) on macOS
  9pfs: fix qemu_mknodat(S_IFSOCK) on macOS
  9pfs: fix wrong encoding of rdev field in Rgetattr on macOS
  9pfs: fix wrong errno being sent to Linux client on macOS host
  9pfs: fix removing non-existent POSIX ACL xattr on macOS host
  9pfs: fix qemu_mknodat() to always return -1 on error on macOS host

 hw/9pfs/9p-posix-acl.c   | 12 +++++--
 hw/9pfs/9p-util-darwin.c | 54 +++++++++++++++++++++++++++++--
 hw/9pfs/9p-util.h        | 69 ++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/9p.c             |  4 ++-
 4 files changed, 134 insertions(+), 5 deletions(-)

-- 
2.32.0 (Apple Git-132)

Re: [PATCH v5 4/6] 9pfs: fix wrong errno being sent to Linux client on macOS host

[Bin Meng]

On Fri, Apr 29, 2022 at 7:16 PM 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 'Rlerror' replies (if 9p2000.L is used that is). Not
> having translated errnos before violated the 9p2000.L protocol spec,
> which says:
>
>   "
>   size[4] Rlerror tag[2] ecode[4]
>
>   ... ecode is a numerical Linux errno.
>   "
>
>   https://github.com/chaos/diod/wiki/protocol#lerror----return-error-code
>
> This patch fixes a bunch of misbehaviours when running a Linux client
> on macOS host. 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.

This issue looks exact the same issue we were trying to fix when
supporting 9p on Windows host,

What we did is like this:
http://patchwork.ozlabs.org/project/qemu-devel/patch/20220425142705.2099270-10-bmeng.cn@gmail.com/

But we had some questions in mind (see the commit message of our
patch, and below)

>
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> Link: https://lore.kernel.org/qemu-devel/20220421124835.3e664669@bahia/
> Reviewed-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/9pfs/9p-util.h | 30 ++++++++++++++++++++++++++++++
>  hw/9pfs/9p.c      |  2 ++
>  2 files changed, 32 insertions(+)
>
> diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> index 2cc9a5dbfb..c3526144c9 100644
> --- a/hw/9pfs/9p-util.h
> +++ b/hw/9pfs/9p-util.h
> @@ -58,6 +58,36 @@ static inline uint64_t host_dev_to_dotl_dev(dev_t dev)
>  #endif
>  }
>
> +/* Translates errno from host -> Linux if needed */
> +static inline int errno_to_dotl(int err) {
> +#if defined(CONFIG_LINUX)
> +    /* nothing to translate (Linux -> Linux) */
> +#elif defined(CONFIG_DARWIN)
> +    /*
> +     * translation mandatory for macOS hosts
> +     *
> +     * FIXME: Only most important errnos translated here yet, this should be
> +     * extended to as many errnos being translated as possible in future.
> +     */
> +    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 */
> +    }

What happens if a macOS guest is running on QEMU from a macOS host?
Here all macOS errnos are translated to the Linux errnos. Will macOS
be happy?

> +#else
> +#error Missing errno translation to Linux for this host system
> +#endif
> +    return err;
> +}
> +
>  #ifdef CONFIG_DARWIN
>  #define qemu_fgetxattr(...) fgetxattr(__VA_ARGS__, 0, 0)
>  #define qemu_lgetxattr(...) getxattr(__VA_ARGS__, 0, XATTR_NOFOLLOW)
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 4a296a0b94..0cd0c14c2a 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1054,6 +1054,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);
> --

Regards,
Bin

Re: [PATCH v5 4/6] 9pfs: fix wrong errno being sent to Linux client on macOS host

[Greg Kurz]

On Fri, 29 Apr 2022 19:28:39 +0800
Bin Meng <bmeng.cn@gmail.com> wrote:

> On Fri, Apr 29, 2022 at 7:16 PM 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 'Rlerror' replies (if 9p2000.L is used that is). Not
> > having translated errnos before violated the 9p2000.L protocol spec,
> > which says:
> >
> >   "
> >   size[4] Rlerror tag[2] ecode[4]
> >
> >   ... ecode is a numerical Linux errno.
> >   "
> >
> >   https://github.com/chaos/diod/wiki/protocol#lerror----return-error-code
> >
> > This patch fixes a bunch of misbehaviours when running a Linux client
> > on macOS host. 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.
> 
> This issue looks exact the same issue we were trying to fix when
> supporting 9p on Windows host,
> 
> What we did is like this:
> http://patchwork.ozlabs.org/project/qemu-devel/patch/20220425142705.2099270-10-bmeng.cn@gmail.com/
> 
> But we had some questions in mind (see the commit message of our
> patch, and below)
> 
> >
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > Link: https://lore.kernel.org/qemu-devel/20220421124835.3e664669@bahia/
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> > ---
> >  hw/9pfs/9p-util.h | 30 ++++++++++++++++++++++++++++++
> >  hw/9pfs/9p.c      |  2 ++
> >  2 files changed, 32 insertions(+)
> >
> > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> > index 2cc9a5dbfb..c3526144c9 100644
> > --- a/hw/9pfs/9p-util.h
> > +++ b/hw/9pfs/9p-util.h
> > @@ -58,6 +58,36 @@ static inline uint64_t host_dev_to_dotl_dev(dev_t dev)
> >  #endif
> >  }
> >
> > +/* Translates errno from host -> Linux if needed */
> > +static inline int errno_to_dotl(int err) {
> > +#if defined(CONFIG_LINUX)
> > +    /* nothing to translate (Linux -> Linux) */
> > +#elif defined(CONFIG_DARWIN)
> > +    /*
> > +     * translation mandatory for macOS hosts
> > +     *
> > +     * FIXME: Only most important errnos translated here yet, this should be
> > +     * extended to as many errnos being translated as possible in future.
> > +     */
> > +    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 */
> > +    }
> 
> What happens if a macOS guest is running on QEMU from a macOS host?
> Here all macOS errnos are translated to the Linux errnos. Will macOS
> be happy?
> 

The 9p2000.L protocol is tailored for a linux client. As mentioned in
the changelog, the spec explicitly mentions 'numerical linux errno'.
A macOS client should thus convert back errno in a Rlerror message
to a macOS errno.

> > +#else
> > +#error Missing errno translation to Linux for this host system
> > +#endif
> > +    return err;
> > +}
> > +
> >  #ifdef CONFIG_DARWIN
> >  #define qemu_fgetxattr(...) fgetxattr(__VA_ARGS__, 0, 0)
> >  #define qemu_lgetxattr(...) getxattr(__VA_ARGS__, 0, XATTR_NOFOLLOW)
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 4a296a0b94..0cd0c14c2a 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -1054,6 +1054,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);
> > --
> 
> Regards,
> Bin

Re: [PATCH v5 4/6] 9pfs: fix wrong errno being sent to Linux client on macOS host

[Christian Schoenebeck]

On Freitag, 29. April 2022 13:28:39 CEST Bin Meng wrote:
> On Fri, Apr 29, 2022 at 7:16 PM 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 'Rlerror' replies (if 9p2000.L is used that is). Not
> > having translated errnos before violated the 9p2000.L protocol spec,
> > 
> > which says:
> >   "
> >   size[4] Rlerror tag[2] ecode[4]
> >   
> >   ... ecode is a numerical Linux errno.
> >   "
> >   
> >   https://github.com/chaos/diod/wiki/protocol#lerror----return-error-code
> > 
> > This patch fixes a bunch of misbehaviours when running a Linux client
> > 
> > on macOS host. 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.
> 
> This issue looks exact the same issue we were trying to fix when
> supporting 9p on Windows host,
> 
> What we did is like this:
> http://patchwork.ozlabs.org/project/qemu-devel/patch/20220425142705.2099270-> 10-bmeng.cn@gmail.com/
> 
> But we had some questions in mind (see the commit message of our
> patch, and below)
> 
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > Link: https://lore.kernel.org/qemu-devel/20220421124835.3e664669@bahia/
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> > ---
> > 
> >  hw/9pfs/9p-util.h | 30 ++++++++++++++++++++++++++++++
> >  hw/9pfs/9p.c      |  2 ++
> >  2 files changed, 32 insertions(+)
> > 
> > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> > index 2cc9a5dbfb..c3526144c9 100644
> > --- a/hw/9pfs/9p-util.h
> > +++ b/hw/9pfs/9p-util.h
> > @@ -58,6 +58,36 @@ static inline uint64_t host_dev_to_dotl_dev(dev_t dev)
> > 
> >  #endif
> >  }
> > 
> > +/* Translates errno from host -> Linux if needed */
> > +static inline int errno_to_dotl(int err) {
> > +#if defined(CONFIG_LINUX)
> > +    /* nothing to translate (Linux -> Linux) */
> > +#elif defined(CONFIG_DARWIN)
> > +    /*
> > +     * translation mandatory for macOS hosts
> > +     *
> > +     * FIXME: Only most important errnos translated here yet, this should
> > be +     * extended to as many errnos being translated as possible in
> > future. +     */
> > +    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 */
> > +    }
> 
> What happens if a macOS guest is running on QEMU from a macOS host?
> Here all macOS errnos are translated to the Linux errnos. Will macOS
> be happy?

Look at the commit log of this patch: it is a matter of which protocol is used 
(currently there are 3 [1] protocol versions):

   * 9p2000: nothing to translate here, as this protocol version does not
     return a numeric error code, it only returns an error string (and we are 
     no longer supporting 9p2000 version in QEMU anyway BTW [1]):
     http://ericvh.github.io/9p-rfc/rfc9p2000.html#anchor27

   * 9p2000.L: errno *must* be in Linux errno mapping:
     https://github.com/chaos/diod/wiki/protocol#lerror----return-error-code

   * 9p2000.u: this one returns both an error code and error string, and it 
     says the error string should be preferred being interpreted by client:
     http://ericvh.github.io/9p-rfc/rfc9p2000.u.html#anchor15

In this patch here I only translated errno for 9p2000.L, whereas you are 
always translating it, no matter wich protocol version is used. You might 
argue that there should be a translation for 9p2000.u as well, but in the end 
we don't know the OS running on guest in this case. It could be Linux or 
something else.

[1] https://wiki.qemu.org/Documentation/9p#9p_Protocol

Best regards,
Christian Schoenebeck

> 
> > +#else
> > +#error Missing errno translation to Linux for this host system
> > +#endif
> > +    return err;
> > +}
> > +
> > 
> >  #ifdef CONFIG_DARWIN
> >  #define qemu_fgetxattr(...) fgetxattr(__VA_ARGS__, 0, 0)
> >  #define qemu_lgetxattr(...) getxattr(__VA_ARGS__, 0, XATTR_NOFOLLOW)
> > 
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 4a296a0b94..0cd0c14c2a 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -1054,6 +1054,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);
> > 
> > --
> 
> Regards,
> Bin

Re: [PATCH v5 2/6] 9pfs: fix qemu_mknodat(S_IFSOCK) on macOS

[Greg Kurz]

On Fri, 29 Apr 2022 12:25:11 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> mknod() on macOS does not support creating sockets, so divert to
> call sequence socket(), bind() and fchmodat() respectively if S_IFSOCK
> was passed with mode argument.
> 
> Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---
>  hw/9pfs/9p-util-darwin.c | 42 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> index e24d09763a..619c403ba7 100644
> --- a/hw/9pfs/9p-util-darwin.c
> +++ b/hw/9pfs/9p-util-darwin.c
> @@ -74,6 +74,42 @@ int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
>   */
>  #if defined CONFIG_PTHREAD_FCHDIR_NP
>  
> +static int create_socket_file_at_cwd(const char *filename, mode_t mode) {
> +    int fd, err;
> +    struct sockaddr_un addr = {
> +        .sun_family = AF_UNIX
> +    };
> +
> +    err = snprintf(addr.sun_path, sizeof(addr.sun_path), "./%s", filename);
> +    if (err < 0 || err >= sizeof(addr.sun_path)) {

According to POSIX [1]:

The snprintf() function shall fail if:

[EOVERFLOW]
[CX] [Option Start] The value of n is greater than {INT_MAX}. [Option End]

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.html

Since we're passing sizeof(addr.sun_path), I'm pretty sure snprintf()
cannot fail. No big deal.

Reviewed-by: Greg Kurz <groug@kaod.org>

> +        errno = ENAMETOOLONG;
> +        return -1;
> +    }
> +    fd = socket(PF_UNIX, SOCK_DGRAM, 0);
> +    if (fd == -1) {
> +        return fd;
> +    }
> +    err = bind(fd, (struct sockaddr *) &addr, sizeof(addr));
> +    if (err == -1) {
> +        goto out;
> +    }
> +    /*
> +     * FIXME: Should rather be using descriptor-based fchmod() on the
> +     * socket file descriptor above (preferably before bind() call),
> +     * instead of path-based fchmodat(), to prevent concurrent transient
> +     * state issues between creating the named FIFO file at bind() and
> +     * delayed adjustment of permissions at fchmodat(). However currently
> +     * macOS (12.x) does not support such operations on socket file
> +     * descriptors yet.
> +     *
> +     * Filed report with Apple: FB9997731
> +     */
> +    err = fchmodat(AT_FDCWD, filename, mode, AT_SYMLINK_NOFOLLOW_ANY);
> +out:
> +    close_preserve_errno(fd);
> +    return err;
> +}
> +
>  int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
>  {
>      int preserved_errno, err;
> @@ -93,7 +129,11 @@ int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
>      if (pthread_fchdir_np(dirfd) < 0) {
>          return -1;
>      }
> -    err = mknod(filename, mode, dev);
> +    if (S_ISSOCK(mode)) {
> +        err = create_socket_file_at_cwd(filename, mode);
> +    } else {
> +        err = mknod(filename, mode, dev);
> +    }
>      preserved_errno = errno;
>      /* Stop using the thread-local cwd */
>      pthread_fchdir_np(-1);

Re: [PATCH v5 4/6] 9pfs: fix wrong errno being sent to Linux client on macOS host

[Greg Kurz]

On Fri, 29 Apr 2022 14:46:26 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Freitag, 29. April 2022 13:28:39 CEST Bin Meng wrote:
> > On Fri, Apr 29, 2022 at 7:16 PM 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 'Rlerror' replies (if 9p2000.L is used that is). Not
> > > having translated errnos before violated the 9p2000.L protocol spec,
> > > 
> > > which says:
> > >   "
> > >   size[4] Rlerror tag[2] ecode[4]
> > >   
> > >   ... ecode is a numerical Linux errno.
> > >   "
> > >   
> > >   https://github.com/chaos/diod/wiki/protocol#lerror----return-error-code
> > > 
> > > This patch fixes a bunch of misbehaviours when running a Linux client
> > > 
> > > on macOS host. 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.
> > 
> > This issue looks exact the same issue we were trying to fix when
> > supporting 9p on Windows host,
> > 
> > What we did is like this:
> > http://patchwork.ozlabs.org/project/qemu-devel/patch/20220425142705.2099270-> 10-bmeng.cn@gmail.com/
> > 
> > But we had some questions in mind (see the commit message of our
> > patch, and below)
> > 
> > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > Link: https://lore.kernel.org/qemu-devel/20220421124835.3e664669@bahia/
> > > Reviewed-by: Greg Kurz <groug@kaod.org>
> > > ---
> > > 
> > >  hw/9pfs/9p-util.h | 30 ++++++++++++++++++++++++++++++
> > >  hw/9pfs/9p.c      |  2 ++
> > >  2 files changed, 32 insertions(+)
> > > 
> > > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> > > index 2cc9a5dbfb..c3526144c9 100644
> > > --- a/hw/9pfs/9p-util.h
> > > +++ b/hw/9pfs/9p-util.h
> > > @@ -58,6 +58,36 @@ static inline uint64_t host_dev_to_dotl_dev(dev_t dev)
> > > 
> > >  #endif
> > >  }
> > > 
> > > +/* Translates errno from host -> Linux if needed */
> > > +static inline int errno_to_dotl(int err) {
> > > +#if defined(CONFIG_LINUX)
> > > +    /* nothing to translate (Linux -> Linux) */
> > > +#elif defined(CONFIG_DARWIN)
> > > +    /*
> > > +     * translation mandatory for macOS hosts
> > > +     *
> > > +     * FIXME: Only most important errnos translated here yet, this should
> > > be +     * extended to as many errnos being translated as possible in
> > > future. +     */
> > > +    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 */
> > > +    }
> > 
> > What happens if a macOS guest is running on QEMU from a macOS host?
> > Here all macOS errnos are translated to the Linux errnos. Will macOS
> > be happy?
> 
> Look at the commit log of this patch: it is a matter of which protocol is used 
> (currently there are 3 [1] protocol versions):
> 
>    * 9p2000: nothing to translate here, as this protocol version does not
>      return a numeric error code, it only returns an error string (and we are 
>      no longer supporting 9p2000 version in QEMU anyway BTW [1]):
>      http://ericvh.github.io/9p-rfc/rfc9p2000.html#anchor27
> 
>    * 9p2000.L: errno *must* be in Linux errno mapping:
>      https://github.com/chaos/diod/wiki/protocol#lerror----return-error-code
> 
>    * 9p2000.u: this one returns both an error code and error string, and it 
>      says the error string should be preferred being interpreted by client:
>      http://ericvh.github.io/9p-rfc/rfc9p2000.u.html#anchor15
> 
> In this patch here I only translated errno for 9p2000.L, whereas you are 
> always translating it, no matter wich protocol version is used. You might 
> argue that there should be a translation for 9p2000.u as well, but in the end 
> we don't know the OS running on guest in this case. It could be Linux or 
> something else.
> 

In the case of 9p2000.u the spec says "to provide a hint of the underlying
UNIX error number which caused the error on the server" and even mentions
"consistency problems of mapping error numbers betweeen different versions
of UNIX"... this basically means that errno in 9p2000.u is undefined since
it depends on the host. It is thus unusable unless the guest runs a compatible
UNIX variant. In any case, there's really nothing to translate.

> [1] https://wiki.qemu.org/Documentation/9p#9p_Protocol
> 
> Best regards,
> Christian Schoenebeck
> 
> > 
> > > +#else
> > > +#error Missing errno translation to Linux for this host system
> > > +#endif
> > > +    return err;
> > > +}
> > > +
> > > 
> > >  #ifdef CONFIG_DARWIN
> > >  #define qemu_fgetxattr(...) fgetxattr(__VA_ARGS__, 0, 0)
> > >  #define qemu_lgetxattr(...) getxattr(__VA_ARGS__, 0, XATTR_NOFOLLOW)
> > > 
> > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > > index 4a296a0b94..0cd0c14c2a 100644
> > > --- a/hw/9pfs/9p.c
> > > +++ b/hw/9pfs/9p.c
> > > @@ -1054,6 +1054,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);
> > > 
> > > --
> > 
> > Regards,
> > Bin
> 
> 

Re: [PATCH v5 4/6] 9pfs: fix wrong errno being sent to Linux client on macOS host

[Bin Meng]

On Fri, Apr 29, 2022 at 9:08 PM Greg Kurz <groug@kaod.org> wrote:
>
> On Fri, 29 Apr 2022 14:46:26 +0200
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
>
> > On Freitag, 29. April 2022 13:28:39 CEST Bin Meng wrote:
> > > On Fri, Apr 29, 2022 at 7:16 PM 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 'Rlerror' replies (if 9p2000.L is used that is). Not
> > > > having translated errnos before violated the 9p2000.L protocol spec,
> > > >
> > > > which says:
> > > >   "
> > > >   size[4] Rlerror tag[2] ecode[4]
> > > >
> > > >   ... ecode is a numerical Linux errno.
> > > >   "
> > > >
> > > >   https://github.com/chaos/diod/wiki/protocol#lerror----return-error-code
> > > >
> > > > This patch fixes a bunch of misbehaviours when running a Linux client
> > > >
> > > > on macOS host. 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.
> > >
> > > This issue looks exact the same issue we were trying to fix when
> > > supporting 9p on Windows host,
> > >
> > > What we did is like this:
> > > http://patchwork.ozlabs.org/project/qemu-devel/patch/20220425142705.2099270-> 10-bmeng.cn@gmail.com/
> > >
> > > But we had some questions in mind (see the commit message of our
> > > patch, and below)
> > >
> > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > > Link: https://lore.kernel.org/qemu-devel/20220421124835.3e664669@bahia/
> > > > Reviewed-by: Greg Kurz <groug@kaod.org>
> > > > ---
> > > >
> > > >  hw/9pfs/9p-util.h | 30 ++++++++++++++++++++++++++++++
> > > >  hw/9pfs/9p.c      |  2 ++
> > > >  2 files changed, 32 insertions(+)
> > > >
> > > > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> > > > index 2cc9a5dbfb..c3526144c9 100644
> > > > --- a/hw/9pfs/9p-util.h
> > > > +++ b/hw/9pfs/9p-util.h
> > > > @@ -58,6 +58,36 @@ static inline uint64_t host_dev_to_dotl_dev(dev_t dev)
> > > >
> > > >  #endif
> > > >  }
> > > >
> > > > +/* Translates errno from host -> Linux if needed */
> > > > +static inline int errno_to_dotl(int err) {
> > > > +#if defined(CONFIG_LINUX)
> > > > +    /* nothing to translate (Linux -> Linux) */
> > > > +#elif defined(CONFIG_DARWIN)
> > > > +    /*
> > > > +     * translation mandatory for macOS hosts
> > > > +     *
> > > > +     * FIXME: Only most important errnos translated here yet, this should
> > > > be +     * extended to as many errnos being translated as possible in
> > > > future. +     */
> > > > +    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 */
> > > > +    }
> > >
> > > What happens if a macOS guest is running on QEMU from a macOS host?
> > > Here all macOS errnos are translated to the Linux errnos. Will macOS
> > > be happy?
> >
> > Look at the commit log of this patch: it is a matter of which protocol is used
> > (currently there are 3 [1] protocol versions):
> >
> >    * 9p2000: nothing to translate here, as this protocol version does not
> >      return a numeric error code, it only returns an error string (and we are
> >      no longer supporting 9p2000 version in QEMU anyway BTW [1]):
> >      http://ericvh.github.io/9p-rfc/rfc9p2000.html#anchor27
> >
> >    * 9p2000.L: errno *must* be in Linux errno mapping:
> >      https://github.com/chaos/diod/wiki/protocol#lerror----return-error-code
> >
> >    * 9p2000.u: this one returns both an error code and error string, and it
> >      says the error string should be preferred being interpreted by client:
> >      http://ericvh.github.io/9p-rfc/rfc9p2000.u.html#anchor15
> >
> > In this patch here I only translated errno for 9p2000.L, whereas you are
> > always translating it, no matter wich protocol version is used. You might
> > argue that there should be a translation for 9p2000.u as well, but in the end
> > we don't know the OS running on guest in this case. It could be Linux or
> > something else.
> >
>
> In the case of 9p2000.u the spec says "to provide a hint of the underlying
> UNIX error number which caused the error on the server" and even mentions
> "consistency problems of mapping error numbers betweeen different versions
> of UNIX"... this basically means that errno in 9p2000.u is undefined since
> it depends on the host. It is thus unusable unless the guest runs a compatible
> UNIX variant. In any case, there's really nothing to translate.
>
> > [1] https://wiki.qemu.org/Documentation/9p#9p_Protocol
> >

Thanks for the clarifications and pointers to different protocols! It
looks what we did in our Windows patch is correct.

I have another question, does this mean the current 9pfs client on
macOS is broken since it does not use any translation? With this
patch, now the 9p server returns the translated linux errno so the 9p
client on macOS should complain.

Regards,
Bin

Re: [PATCH v5 4/6] 9pfs: fix wrong errno being sent to Linux client on macOS host

[Greg Kurz]

On Fri, 29 Apr 2022 21:19:51 +0800
Bin Meng <bmeng.cn@gmail.com> wrote:

> On Fri, Apr 29, 2022 at 9:08 PM Greg Kurz <groug@kaod.org> wrote:
> >
> > On Fri, 29 Apr 2022 14:46:26 +0200
> > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> >
> > > On Freitag, 29. April 2022 13:28:39 CEST Bin Meng wrote:
> > > > On Fri, Apr 29, 2022 at 7:16 PM 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 'Rlerror' replies (if 9p2000.L is used that is). Not
> > > > > having translated errnos before violated the 9p2000.L protocol spec,
> > > > >
> > > > > which says:
> > > > >   "
> > > > >   size[4] Rlerror tag[2] ecode[4]
> > > > >
> > > > >   ... ecode is a numerical Linux errno.
> > > > >   "
> > > > >
> > > > >   https://github.com/chaos/diod/wiki/protocol#lerror----return-error-code
> > > > >
> > > > > This patch fixes a bunch of misbehaviours when running a Linux client
> > > > >
> > > > > on macOS host. 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.
> > > >
> > > > This issue looks exact the same issue we were trying to fix when
> > > > supporting 9p on Windows host,
> > > >
> > > > What we did is like this:
> > > > http://patchwork.ozlabs.org/project/qemu-devel/patch/20220425142705.2099270-> 10-bmeng.cn@gmail.com/
> > > >
> > > > But we had some questions in mind (see the commit message of our
> > > > patch, and below)
> > > >
> > > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > > > Link: https://lore.kernel.org/qemu-devel/20220421124835.3e664669@bahia/
> > > > > Reviewed-by: Greg Kurz <groug@kaod.org>
> > > > > ---
> > > > >
> > > > >  hw/9pfs/9p-util.h | 30 ++++++++++++++++++++++++++++++
> > > > >  hw/9pfs/9p.c      |  2 ++
> > > > >  2 files changed, 32 insertions(+)
> > > > >
> > > > > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> > > > > index 2cc9a5dbfb..c3526144c9 100644
> > > > > --- a/hw/9pfs/9p-util.h
> > > > > +++ b/hw/9pfs/9p-util.h
> > > > > @@ -58,6 +58,36 @@ static inline uint64_t host_dev_to_dotl_dev(dev_t dev)
> > > > >
> > > > >  #endif
> > > > >  }
> > > > >
> > > > > +/* Translates errno from host -> Linux if needed */
> > > > > +static inline int errno_to_dotl(int err) {
> > > > > +#if defined(CONFIG_LINUX)
> > > > > +    /* nothing to translate (Linux -> Linux) */
> > > > > +#elif defined(CONFIG_DARWIN)
> > > > > +    /*
> > > > > +     * translation mandatory for macOS hosts
> > > > > +     *
> > > > > +     * FIXME: Only most important errnos translated here yet, this should
> > > > > be +     * extended to as many errnos being translated as possible in
> > > > > future. +     */
> > > > > +    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 */
> > > > > +    }
> > > >
> > > > What happens if a macOS guest is running on QEMU from a macOS host?
> > > > Here all macOS errnos are translated to the Linux errnos. Will macOS
> > > > be happy?
> > >
> > > Look at the commit log of this patch: it is a matter of which protocol is used
> > > (currently there are 3 [1] protocol versions):
> > >
> > >    * 9p2000: nothing to translate here, as this protocol version does not
> > >      return a numeric error code, it only returns an error string (and we are
> > >      no longer supporting 9p2000 version in QEMU anyway BTW [1]):
> > >      http://ericvh.github.io/9p-rfc/rfc9p2000.html#anchor27
> > >
> > >    * 9p2000.L: errno *must* be in Linux errno mapping:
> > >      https://github.com/chaos/diod/wiki/protocol#lerror----return-error-code
> > >
> > >    * 9p2000.u: this one returns both an error code and error string, and it
> > >      says the error string should be preferred being interpreted by client:
> > >      http://ericvh.github.io/9p-rfc/rfc9p2000.u.html#anchor15
> > >
> > > In this patch here I only translated errno for 9p2000.L, whereas you are
> > > always translating it, no matter wich protocol version is used. You might
> > > argue that there should be a translation for 9p2000.u as well, but in the end
> > > we don't know the OS running on guest in this case. It could be Linux or
> > > something else.
> > >
> >
> > In the case of 9p2000.u the spec says "to provide a hint of the underlying
> > UNIX error number which caused the error on the server" and even mentions
> > "consistency problems of mapping error numbers betweeen different versions
> > of UNIX"... this basically means that errno in 9p2000.u is undefined since
> > it depends on the host. It is thus unusable unless the guest runs a compatible
> > UNIX variant. In any case, there's really nothing to translate.
> >
> > > [1] https://wiki.qemu.org/Documentation/9p#9p_Protocol
> > >
> 
> Thanks for the clarifications and pointers to different protocols! It
> looks what we did in our Windows patch is correct.
> 
> I have another question, does this mean the current 9pfs client on
> macOS is broken since it does not use any translation? With this
> patch, now the 9p server returns the translated linux errno so the 9p
> client on macOS should complain.
> 

I don't now the macOS client but if it doesn't expect linux errnos
then it is infringing 9p2000.L and should be fixed.

> Regards,
> Bin

Re: [PATCH v5 4/6] 9pfs: fix wrong errno being sent to Linux client on macOS host

[Christian Schoenebeck]

On Freitag, 29. April 2022 15:29:15 CEST Greg Kurz wrote:
> On Fri, 29 Apr 2022 21:19:51 +0800
> 
> Bin Meng <bmeng.cn@gmail.com> wrote:
> > On Fri, Apr 29, 2022 at 9:08 PM Greg Kurz <groug@kaod.org> wrote:
> > > On Fri, 29 Apr 2022 14:46:26 +0200
> > > 
> > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > > On Freitag, 29. April 2022 13:28:39 CEST Bin Meng wrote:
> > > > > On Fri, Apr 29, 2022 at 7:16 PM 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 'Rlerror' replies (if 9p2000.L is used that is).
> > > > > > Not
> > > > > > having translated errnos before violated the 9p2000.L protocol
> > > > > > spec,
> > > > > > 
> > > > > > which says:
> > > > > >   "
> > > > > >   size[4] Rlerror tag[2] ecode[4]
> > > > > >   
> > > > > >   ... ecode is a numerical Linux errno.
> > > > > >   "
> > > > > >   
> > > > > >   https://github.com/chaos/diod/wiki/protocol#lerror----return-err
> > > > > >   or-code
> > > > > > 
> > > > > > This patch fixes a bunch of misbehaviours when running a Linux
> > > > > > client
> > > > > > 
> > > > > > on macOS host. 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.
> > > > > 
> > > > > This issue looks exact the same issue we were trying to fix when
> > > > > supporting 9p on Windows host,
> > > > > 
> > > > > What we did is like this:
> > > > > http://patchwork.ozlabs.org/project/qemu-devel/patch/20220425142705.
> > > > > 2099270-> 10-bmeng.cn@gmail.com/
> > > > > 
> > > > > But we had some questions in mind (see the commit message of our
> > > > > patch, and below)
> > > > > 
> > > > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > > > > Link:
> > > > > > https://lore.kernel.org/qemu-devel/20220421124835.3e664669@bahia/
> > > > > > Reviewed-by: Greg Kurz <groug@kaod.org>
> > > > > > ---
> > > > > > 
> > > > > >  hw/9pfs/9p-util.h | 30 ++++++++++++++++++++++++++++++
> > > > > >  hw/9pfs/9p.c      |  2 ++
> > > > > >  2 files changed, 32 insertions(+)
> > > > > > 
> > > > > > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> > > > > > index 2cc9a5dbfb..c3526144c9 100644
> > > > > > --- a/hw/9pfs/9p-util.h
> > > > > > +++ b/hw/9pfs/9p-util.h
> > > > > > @@ -58,6 +58,36 @@ static inline uint64_t
> > > > > > host_dev_to_dotl_dev(dev_t dev)
> > > > > > 
> > > > > >  #endif
> > > > > >  }
> > > > > > 
> > > > > > +/* Translates errno from host -> Linux if needed */
> > > > > > +static inline int errno_to_dotl(int err) {
> > > > > > +#if defined(CONFIG_LINUX)
> > > > > > +    /* nothing to translate (Linux -> Linux) */
> > > > > > +#elif defined(CONFIG_DARWIN)
> > > > > > +    /*
> > > > > > +     * translation mandatory for macOS hosts
> > > > > > +     *
> > > > > > +     * FIXME: Only most important errnos translated here yet,
> > > > > > this should
> > > > > > be +     * extended to as many errnos being translated as possible
> > > > > > in
> > > > > > future. +     */
> > > > > > +    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 */
> > > > > > +    }
> > > > > 
> > > > > What happens if a macOS guest is running on QEMU from a macOS host?
> > > > > Here all macOS errnos are translated to the Linux errnos. Will macOS
> > > > > be happy?
> > > > 
> > > > Look at the commit log of this patch: it is a matter of which protocol
> > > > is used> > > 
> > > > (currently there are 3 [1] protocol versions):
> > > >    * 9p2000: nothing to translate here, as this protocol version does
> > > >    not
> > > >    
> > > >      return a numeric error code, it only returns an error string (and
> > > >      we are
> > > >      no longer supporting 9p2000 version in QEMU anyway BTW [1]):
> > > >      http://ericvh.github.io/9p-rfc/rfc9p2000.html#anchor27
> > > >    
> > > >    * 9p2000.L: errno *must* be in Linux errno mapping:
> > > >      https://github.com/chaos/diod/wiki/protocol#lerror----return-erro
> > > >      r-code
> > > >    
> > > >    * 9p2000.u: this one returns both an error code and error string,
> > > >    and it
> > > >    
> > > >      says the error string should be preferred being interpreted by
> > > >      client:
> > > >      http://ericvh.github.io/9p-rfc/rfc9p2000.u.html#anchor15
> > > > 
> > > > In this patch here I only translated errno for 9p2000.L, whereas you
> > > > are
> > > > always translating it, no matter wich protocol version is used. You
> > > > might
> > > > argue that there should be a translation for 9p2000.u as well, but in
> > > > the end we don't know the OS running on guest in this case. It could
> > > > be Linux or something else.
> > > 
> > > In the case of 9p2000.u the spec says "to provide a hint of the
> > > underlying
> > > UNIX error number which caused the error on the server" and even
> > > mentions
> > > "consistency problems of mapping error numbers betweeen different
> > > versions
> > > of UNIX"... this basically means that errno in 9p2000.u is undefined
> > > since
> > > it depends on the host. It is thus unusable unless the guest runs a
> > > compatible UNIX variant. In any case, there's really nothing to
> > > translate.
> > > 
> > > > [1] https://wiki.qemu.org/Documentation/9p#9p_Protocol
> > 
> > Thanks for the clarifications and pointers to different protocols! It
> > looks what we did in our Windows patch is correct.

Like I said, you are translating it for all protocol version, whereas this 
patch here translates errnos for 9p2000.L version only.

> > I have another question, does this mean the current 9pfs client on
> > macOS is broken since it does not use any translation? With this
> > patch, now the 9p server returns the translated linux errno so the 9p
> > client on macOS should complain.
> 
> I don't now the macOS client but if it doesn't expect linux errnos
> then it is infringing 9p2000.L and should be fixed.

Agreed, if you are using 9p2000.L with that macOS client and client does not 
translate errnos Linux -> macOS, then client is broken. In the end it matters 
what the protocol documentation specified.

Which client is that? Is it from Apple or from a third party? And are you sure 
you were actually using 9p2000.L and not 9p2000.u?

Best regards,
Christian Schoenebeck

Re: [PATCH v5 2/6] 9pfs: fix qemu_mknodat(S_IFSOCK) on macOS

[Christian Schoenebeck]

On Freitag, 29. April 2022 14:56:50 CEST Greg Kurz wrote:
> On Fri, 29 Apr 2022 12:25:11 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > mknod() on macOS does not support creating sockets, so divert to
> > call sequence socket(), bind() and fchmodat() respectively if S_IFSOCK
> > was passed with mode argument.
> > 
> > Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > ---
> > 
> >  hw/9pfs/9p-util-darwin.c | 42 +++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 41 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> > index e24d09763a..619c403ba7 100644
> > --- a/hw/9pfs/9p-util-darwin.c
> > +++ b/hw/9pfs/9p-util-darwin.c
> > @@ -74,6 +74,42 @@ int fsetxattrat_nofollow(int dirfd, const char
> > *filename, const char *name,> 
> >   */
> >  
> >  #if defined CONFIG_PTHREAD_FCHDIR_NP
> > 
> > +static int create_socket_file_at_cwd(const char *filename, mode_t mode) {
> > +    int fd, err;
> > +    struct sockaddr_un addr = {
> > +        .sun_family = AF_UNIX
> > +    };
> > +
> > +    err = snprintf(addr.sun_path, sizeof(addr.sun_path), "./%s",
> > filename); +    if (err < 0 || err >= sizeof(addr.sun_path)) {
> 
> According to POSIX [1]:
> 
> The snprintf() function shall fail if:
> 
> [EOVERFLOW]
> [CX] [Option Start] The value of n is greater than {INT_MAX}. [Option End]
> 
> [1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.html
> 
> Since we're passing sizeof(addr.sun_path), I'm pretty sure snprintf()
> cannot fail. No big deal.

The question is whom you would want to trust on this? POSIX? ISO-C? Clang? 
BSD? Apple? And for how long into future? I mean in general yes, I would not 
expect it to fail with -1 here either, but there are various different API 
docs on snprintf() out there, and most of them don't even bother to enumarate 
which encoding errors may happen. And I'm pretty sure if I'd drop the negative 
err check here, then Akihiko would slap me for unforeseeable additional error 
cases on snprintf() that may be added in future.

Apple's documentation on snprintf() BTW just says:

  "These functions return a negative value if an error occurs."

So Apple does not even restrict the return value to -1 on errrors, you would 
also need to expect other negative values.

So on doubt, I leave this negative result check for now. ;-)

> Reviewed-by: Greg Kurz <groug@kaod.org>

Thanks!

Best regards
Christian Schoenebeck

> > +        errno = ENAMETOOLONG;
> > +        return -1;
> > +    }
> > +    fd = socket(PF_UNIX, SOCK_DGRAM, 0);
> > +    if (fd == -1) {
> > +        return fd;
> > +    }
> > +    err = bind(fd, (struct sockaddr *) &addr, sizeof(addr));
> > +    if (err == -1) {
> > +        goto out;
> > +    }
> > +    /*
> > +     * FIXME: Should rather be using descriptor-based fchmod() on the
> > +     * socket file descriptor above (preferably before bind() call),
> > +     * instead of path-based fchmodat(), to prevent concurrent transient
> > +     * state issues between creating the named FIFO file at bind() and
> > +     * delayed adjustment of permissions at fchmodat(). However currently
> > +     * macOS (12.x) does not support such operations on socket file
> > +     * descriptors yet.
> > +     *
> > +     * Filed report with Apple: FB9997731
> > +     */
> > +    err = fchmodat(AT_FDCWD, filename, mode, AT_SYMLINK_NOFOLLOW_ANY);
> > +out:
> > +    close_preserve_errno(fd);
> > +    return err;
> > +}
> > +
> > 
> >  int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
> >  {
> >  
> >      int preserved_errno, err;
> > 
> > @@ -93,7 +129,11 @@ int qemu_mknodat(int dirfd, const char *filename,
> > mode_t mode, dev_t dev)> 
> >      if (pthread_fchdir_np(dirfd) < 0) {
> >      
> >          return -1;
> >      
> >      }
> > 
> > -    err = mknod(filename, mode, dev);
> > +    if (S_ISSOCK(mode)) {
> > +        err = create_socket_file_at_cwd(filename, mode);
> > +    } else {
> > +        err = mknod(filename, mode, dev);
> > +    }
> > 
> >      preserved_errno = errno;
> >      /* Stop using the thread-local cwd */
> >      pthread_fchdir_np(-1);

Re: [PATCH v5 4/6] 9pfs: fix wrong errno being sent to Linux client on macOS host

[Bin Meng]

On Fri, Apr 29, 2022 at 9:48 PM Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
>
> On Freitag, 29. April 2022 15:29:15 CEST Greg Kurz wrote:
> > On Fri, 29 Apr 2022 21:19:51 +0800
> >
> > Bin Meng <bmeng.cn@gmail.com> wrote:
> > > On Fri, Apr 29, 2022 at 9:08 PM Greg Kurz <groug@kaod.org> wrote:
> > > > On Fri, 29 Apr 2022 14:46:26 +0200
> > > >
> > > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > > > On Freitag, 29. April 2022 13:28:39 CEST Bin Meng wrote:
> > > > > > On Fri, Apr 29, 2022 at 7:16 PM 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 'Rlerror' replies (if 9p2000.L is used that is).
> > > > > > > Not
> > > > > > > having translated errnos before violated the 9p2000.L protocol
> > > > > > > spec,
> > > > > > >
> > > > > > > which says:
> > > > > > >   "
> > > > > > >   size[4] Rlerror tag[2] ecode[4]
> > > > > > >
> > > > > > >   ... ecode is a numerical Linux errno.
> > > > > > >   "
> > > > > > >
> > > > > > >   https://github.com/chaos/diod/wiki/protocol#lerror----return-err
> > > > > > >   or-code
> > > > > > >
> > > > > > > This patch fixes a bunch of misbehaviours when running a Linux
> > > > > > > client
> > > > > > >
> > > > > > > on macOS host. 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.
> > > > > >
> > > > > > This issue looks exact the same issue we were trying to fix when
> > > > > > supporting 9p on Windows host,
> > > > > >
> > > > > > What we did is like this:
> > > > > > http://patchwork.ozlabs.org/project/qemu-devel/patch/20220425142705.
> > > > > > 2099270-> 10-bmeng.cn@gmail.com/
> > > > > >
> > > > > > But we had some questions in mind (see the commit message of our
> > > > > > patch, and below)
> > > > > >
> > > > > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > > > > > Link:
> > > > > > > https://lore.kernel.org/qemu-devel/20220421124835.3e664669@bahia/
> > > > > > > Reviewed-by: Greg Kurz <groug@kaod.org>
> > > > > > > ---
> > > > > > >
> > > > > > >  hw/9pfs/9p-util.h | 30 ++++++++++++++++++++++++++++++
> > > > > > >  hw/9pfs/9p.c      |  2 ++
> > > > > > >  2 files changed, 32 insertions(+)
> > > > > > >
> > > > > > > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> > > > > > > index 2cc9a5dbfb..c3526144c9 100644
> > > > > > > --- a/hw/9pfs/9p-util.h
> > > > > > > +++ b/hw/9pfs/9p-util.h
> > > > > > > @@ -58,6 +58,36 @@ static inline uint64_t
> > > > > > > host_dev_to_dotl_dev(dev_t dev)
> > > > > > >
> > > > > > >  #endif
> > > > > > >  }
> > > > > > >
> > > > > > > +/* Translates errno from host -> Linux if needed */
> > > > > > > +static inline int errno_to_dotl(int err) {
> > > > > > > +#if defined(CONFIG_LINUX)
> > > > > > > +    /* nothing to translate (Linux -> Linux) */
> > > > > > > +#elif defined(CONFIG_DARWIN)
> > > > > > > +    /*
> > > > > > > +     * translation mandatory for macOS hosts
> > > > > > > +     *
> > > > > > > +     * FIXME: Only most important errnos translated here yet,
> > > > > > > this should
> > > > > > > be +     * extended to as many errnos being translated as possible
> > > > > > > in
> > > > > > > future. +     */
> > > > > > > +    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 */
> > > > > > > +    }
> > > > > >
> > > > > > What happens if a macOS guest is running on QEMU from a macOS host?
> > > > > > Here all macOS errnos are translated to the Linux errnos. Will macOS
> > > > > > be happy?
> > > > >
> > > > > Look at the commit log of this patch: it is a matter of which protocol
> > > > > is used> > >
> > > > > (currently there are 3 [1] protocol versions):
> > > > >    * 9p2000: nothing to translate here, as this protocol version does
> > > > >    not
> > > > >
> > > > >      return a numeric error code, it only returns an error string (and
> > > > >      we are
> > > > >      no longer supporting 9p2000 version in QEMU anyway BTW [1]):
> > > > >      http://ericvh.github.io/9p-rfc/rfc9p2000.html#anchor27
> > > > >
> > > > >    * 9p2000.L: errno *must* be in Linux errno mapping:
> > > > >      https://github.com/chaos/diod/wiki/protocol#lerror----return-erro
> > > > >      r-code
> > > > >
> > > > >    * 9p2000.u: this one returns both an error code and error string,
> > > > >    and it
> > > > >
> > > > >      says the error string should be preferred being interpreted by
> > > > >      client:
> > > > >      http://ericvh.github.io/9p-rfc/rfc9p2000.u.html#anchor15
> > > > >
> > > > > In this patch here I only translated errno for 9p2000.L, whereas you
> > > > > are
> > > > > always translating it, no matter wich protocol version is used. You
> > > > > might
> > > > > argue that there should be a translation for 9p2000.u as well, but in
> > > > > the end we don't know the OS running on guest in this case. It could
> > > > > be Linux or something else.
> > > >
> > > > In the case of 9p2000.u the spec says "to provide a hint of the
> > > > underlying
> > > > UNIX error number which caused the error on the server" and even
> > > > mentions
> > > > "consistency problems of mapping error numbers betweeen different
> > > > versions
> > > > of UNIX"... this basically means that errno in 9p2000.u is undefined
> > > > since
> > > > it depends on the host. It is thus unusable unless the guest runs a
> > > > compatible UNIX variant. In any case, there's really nothing to
> > > > translate.
> > > >
> > > > > [1] https://wiki.qemu.org/Documentation/9p#9p_Protocol
> > >
> > > Thanks for the clarifications and pointers to different protocols! It
> > > looks what we did in our Windows patch is correct.
>
> Like I said, you are translating it for all protocol version, whereas this
> patch here translates errnos for 9p2000.L version only.

Yes, this should be fixed in v2.

>
> > > I have another question, does this mean the current 9pfs client on
> > > macOS is broken since it does not use any translation? With this
> > > patch, now the 9p server returns the translated linux errno so the 9p
> > > client on macOS should complain.
> >
> > I don't now the macOS client but if it doesn't expect linux errnos
> > then it is infringing 9p2000.L and should be fixed.
>
> Agreed, if you are using 9p2000.L with that macOS client and client does not
> translate errnos Linux -> macOS, then client is broken. In the end it matters
> what the protocol documentation specified.
>
> Which client is that? Is it from Apple or from a third party? And are you sure
> you were actually using 9p2000.L and not 9p2000.u?
>

I was asking this question because that's something that bothered us
when developing the Windows patch because the errno translation does
not exist in current macOS support, which means when using 9p2000.L:

* if testing a Linux 9 client with a macOS QEMU, it should have been
noticed before due to the errno mismatch which this patch is trying to
fix
* the same error mismatch should happen between a macOS 9p client and
a Linux QEMU
* or maybe only macOS 9p client was tested with a macOS QEMU, because
no errno translation exist, which means the macOS 9p client is broken?

Regards,
Bin

Re: [PATCH v5 2/6] 9pfs: fix qemu_mknodat(S_IFSOCK) on macOS

[Greg Kurz]

On Fri, 29 Apr 2022 15:50:35 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Freitag, 29. April 2022 14:56:50 CEST Greg Kurz wrote:
> > On Fri, 29 Apr 2022 12:25:11 +0200
> > 
> > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > mknod() on macOS does not support creating sockets, so divert to
> > > call sequence socket(), bind() and fchmodat() respectively if S_IFSOCK
> > > was passed with mode argument.
> > > 
> > > Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
> > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > ---
> > > 
> > >  hw/9pfs/9p-util-darwin.c | 42 +++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 41 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> > > index e24d09763a..619c403ba7 100644
> > > --- a/hw/9pfs/9p-util-darwin.c
> > > +++ b/hw/9pfs/9p-util-darwin.c
> > > @@ -74,6 +74,42 @@ int fsetxattrat_nofollow(int dirfd, const char
> > > *filename, const char *name,> 
> > >   */
> > >  
> > >  #if defined CONFIG_PTHREAD_FCHDIR_NP
> > > 
> > > +static int create_socket_file_at_cwd(const char *filename, mode_t mode) {
> > > +    int fd, err;
> > > +    struct sockaddr_un addr = {
> > > +        .sun_family = AF_UNIX
> > > +    };
> > > +
> > > +    err = snprintf(addr.sun_path, sizeof(addr.sun_path), "./%s",
> > > filename); +    if (err < 0 || err >= sizeof(addr.sun_path)) {
> > 
> > According to POSIX [1]:
> > 
> > The snprintf() function shall fail if:
> > 
> > [EOVERFLOW]
> > [CX] [Option Start] The value of n is greater than {INT_MAX}. [Option End]
> > 
> > [1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.html
> > 
> > Since we're passing sizeof(addr.sun_path), I'm pretty sure snprintf()
> > cannot fail. No big deal.
> 
> The question is whom you would want to trust on this? POSIX? ISO-C? Clang? 
> BSD? Apple? And for how long into future? I mean in general yes, I would not 

To improve overall portability across all possible hosts, I'd stick to
POSIX semantics but here this is macOS only code so you can assume
this is Apple's snprintf().

> expect it to fail with -1 here either, but there are various different API 
> docs on snprintf() out there, and most of them don't even bother to enumarate 
> which encoding errors may happen. And I'm pretty sure if I'd drop the negative 
> err check here, then Akihiko would slap me for unforeseeable additional error 
> cases on snprintf() that may be added in future.
> 

/o\ ;-)

> Apple's documentation on snprintf() BTW just says:
> 
>   "These functions return a negative value if an error occurs."
> 

How valuable this is !!! ;-)

> So Apple does not even restrict the return value to -1 on errrors, you would 
> also need to expect other negative values.
> 
> So on doubt, I leave this negative result check for now. ;-)
> 

Fair enough.

> > Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> Thanks!
> 
> Best regards
> Christian Schoenebeck
> 
> > > +        errno = ENAMETOOLONG;
> > > +        return -1;
> > > +    }
> > > +    fd = socket(PF_UNIX, SOCK_DGRAM, 0);
> > > +    if (fd == -1) {
> > > +        return fd;
> > > +    }
> > > +    err = bind(fd, (struct sockaddr *) &addr, sizeof(addr));
> > > +    if (err == -1) {
> > > +        goto out;
> > > +    }
> > > +    /*
> > > +     * FIXME: Should rather be using descriptor-based fchmod() on the
> > > +     * socket file descriptor above (preferably before bind() call),
> > > +     * instead of path-based fchmodat(), to prevent concurrent transient
> > > +     * state issues between creating the named FIFO file at bind() and
> > > +     * delayed adjustment of permissions at fchmodat(). However currently
> > > +     * macOS (12.x) does not support such operations on socket file
> > > +     * descriptors yet.
> > > +     *
> > > +     * Filed report with Apple: FB9997731
> > > +     */
> > > +    err = fchmodat(AT_FDCWD, filename, mode, AT_SYMLINK_NOFOLLOW_ANY);
> > > +out:
> > > +    close_preserve_errno(fd);
> > > +    return err;
> > > +}
> > > +
> > > 
> > >  int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
> > >  {
> > >  
> > >      int preserved_errno, err;
> > > 
> > > @@ -93,7 +129,11 @@ int qemu_mknodat(int dirfd, const char *filename,
> > > mode_t mode, dev_t dev)> 
> > >      if (pthread_fchdir_np(dirfd) < 0) {
> > >      
> > >          return -1;
> > >      
> > >      }
> > > 
> > > -    err = mknod(filename, mode, dev);
> > > +    if (S_ISSOCK(mode)) {
> > > +        err = create_socket_file_at_cwd(filename, mode);
> > > +    } else {
> > > +        err = mknod(filename, mode, dev);
> > > +    }
> > > 
> > >      preserved_errno = errno;
> > >      /* Stop using the thread-local cwd */
> > >      pthread_fchdir_np(-1);
> 
> 

Re: [PATCH v5 4/6] 9pfs: fix wrong errno being sent to Linux client on macOS host

[Christian Schoenebeck]

On Freitag, 29. April 2022 16:16:54 CEST Bin Meng wrote:
> On Fri, Apr 29, 2022 at 9:48 PM Christian Schoenebeck
> 
> <qemu_oss@crudebyte.com> wrote:
> > On Freitag, 29. April 2022 15:29:15 CEST Greg Kurz wrote:
> > > On Fri, 29 Apr 2022 21:19:51 +0800
> > > 
> > > Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > On Fri, Apr 29, 2022 at 9:08 PM Greg Kurz <groug@kaod.org> wrote:
> > > > > On Fri, 29 Apr 2022 14:46:26 +0200
> > > > > 
> > > > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > > > > On Freitag, 29. April 2022 13:28:39 CEST Bin Meng wrote:
> > > > > > > On Fri, Apr 29, 2022 at 7:16 PM 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 'Rlerror' replies (if 9p2000.L is used that
> > > > > > > > is).
> > > > > > > > Not
> > > > > > > > having translated errnos before violated the 9p2000.L protocol
> > > > > > > > spec,
> > > > > > > > 
> > > > > > > > which says:
> > > > > > > >   "
> > > > > > > >   size[4] Rlerror tag[2] ecode[4]
> > > > > > > >   
> > > > > > > >   ... ecode is a numerical Linux errno.
> > > > > > > >   "
> > > > > > > >   
> > > > > > > >   https://github.com/chaos/diod/wiki/protocol#lerror----return
> > > > > > > >   -err
> > > > > > > >   or-code
> > > > > > > > 
> > > > > > > > This patch fixes a bunch of misbehaviours when running a Linux
> > > > > > > > client
> > > > > > > > 
> > > > > > > > on macOS host. 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.
> > > > > > > 
> > > > > > > This issue looks exact the same issue we were trying to fix when
> > > > > > > supporting 9p on Windows host,
> > > > > > > 
> > > > > > > What we did is like this:
> > > > > > > http://patchwork.ozlabs.org/project/qemu-devel/patch/20220425142
> > > > > > > 705.
> > > > > > > 2099270-> 10-bmeng.cn@gmail.com/
> > > > > > > 
> > > > > > > But we had some questions in mind (see the commit message of our
> > > > > > > patch, and below)
> > > > > > > 
> > > > > > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > > > > > > Link:
> > > > > > > > https://lore.kernel.org/qemu-devel/20220421124835.3e664669@bah
> > > > > > > > ia/
> > > > > > > > Reviewed-by: Greg Kurz <groug@kaod.org>
> > > > > > > > ---
> > > > > > > > 
> > > > > > > >  hw/9pfs/9p-util.h | 30 ++++++++++++++++++++++++++++++
> > > > > > > >  hw/9pfs/9p.c      |  2 ++
> > > > > > > >  2 files changed, 32 insertions(+)
> > > > > > > > 
> > > > > > > > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> > > > > > > > index 2cc9a5dbfb..c3526144c9 100644
> > > > > > > > --- a/hw/9pfs/9p-util.h
> > > > > > > > +++ b/hw/9pfs/9p-util.h
> > > > > > > > @@ -58,6 +58,36 @@ static inline uint64_t
> > > > > > > > host_dev_to_dotl_dev(dev_t dev)
> > > > > > > > 
> > > > > > > >  #endif
> > > > > > > >  }
> > > > > > > > 
> > > > > > > > +/* Translates errno from host -> Linux if needed */
> > > > > > > > +static inline int errno_to_dotl(int err) {
> > > > > > > > +#if defined(CONFIG_LINUX)
> > > > > > > > +    /* nothing to translate (Linux -> Linux) */
> > > > > > > > +#elif defined(CONFIG_DARWIN)
> > > > > > > > +    /*
> > > > > > > > +     * translation mandatory for macOS hosts
> > > > > > > > +     *
> > > > > > > > +     * FIXME: Only most important errnos translated here yet,
> > > > > > > > this should
> > > > > > > > be +     * extended to as many errnos being translated as
> > > > > > > > possible
> > > > > > > > in
> > > > > > > > future. +     */
> > > > > > > > +    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 */
> > > > > > > > +    }
> > > > > > > 
> > > > > > > What happens if a macOS guest is running on QEMU from a macOS
> > > > > > > host?
> > > > > > > Here all macOS errnos are translated to the Linux errnos. Will
> > > > > > > macOS
> > > > > > > be happy?
> > > > > > 
> > > > > > Look at the commit log of this patch: it is a matter of which
> > > > > > protocol
> > > > > > is used> > >
> > > > > > 
> > > > > > (currently there are 3 [1] protocol versions):
> > > > > >    * 9p2000: nothing to translate here, as this protocol version
> > > > > >    does
> > > > > >    not
> > > > > >    
> > > > > >      return a numeric error code, it only returns an error string
> > > > > >      (and
> > > > > >      we are
> > > > > >      no longer supporting 9p2000 version in QEMU anyway BTW [1]):
> > > > > >      http://ericvh.github.io/9p-rfc/rfc9p2000.html#anchor27
> > > > > >    
> > > > > >    * 9p2000.L: errno *must* be in Linux errno mapping:
> > > > > >      https://github.com/chaos/diod/wiki/protocol#lerror----return-> > > > > >      erro
> > > > > >      r-code
> > > > > >    
> > > > > >    * 9p2000.u: this one returns both an error code and error
> > > > > >    string,
> > > > > >    and it
> > > > > >    
> > > > > >      says the error string should be preferred being interpreted
> > > > > >      by
> > > > > >      client:
> > > > > >      http://ericvh.github.io/9p-rfc/rfc9p2000.u.html#anchor15
> > > > > > 
> > > > > > In this patch here I only translated errno for 9p2000.L, whereas
> > > > > > you
> > > > > > are
> > > > > > always translating it, no matter wich protocol version is used.
> > > > > > You
> > > > > > might
> > > > > > argue that there should be a translation for 9p2000.u as well, but
> > > > > > in
> > > > > > the end we don't know the OS running on guest in this case. It
> > > > > > could
> > > > > > be Linux or something else.
> > > > > 
> > > > > In the case of 9p2000.u the spec says "to provide a hint of the
> > > > > underlying
> > > > > UNIX error number which caused the error on the server" and even
> > > > > mentions
> > > > > "consistency problems of mapping error numbers betweeen different
> > > > > versions
> > > > > of UNIX"... this basically means that errno in 9p2000.u is undefined
> > > > > since
> > > > > it depends on the host. It is thus unusable unless the guest runs a
> > > > > compatible UNIX variant. In any case, there's really nothing to
> > > > > translate.
> > > > > 
> > > > > > [1] https://wiki.qemu.org/Documentation/9p#9p_Protocol
> > > > 
> > > > Thanks for the clarifications and pointers to different protocols! It
> > > > looks what we did in our Windows patch is correct.
> > 
> > Like I said, you are translating it for all protocol version, whereas this
> > patch here translates errnos for 9p2000.L version only.
> 
> Yes, this should be fixed in v2.

Please wait for posting a v2 of those Windows patches. As you already 
realized, the macOS fix patches here and your Windows host support patches 
collide at several places.

You can roughly expect these macOS patches to be merged on QEMU master branch 
approximately early next week. Once they landed on QEMU master you can rebase 
your patches on top of them and send a v2 accordingly subsequently.

> > > > I have another question, does this mean the current 9pfs client on
> > > > macOS is broken since it does not use any translation? With this
> > > > patch, now the 9p server returns the translated linux errno so the 9p
> > > > client on macOS should complain.
> > > 
> > > I don't now the macOS client but if it doesn't expect linux errnos
> > > then it is infringing 9p2000.L and should be fixed.
> > 
> > Agreed, if you are using 9p2000.L with that macOS client and client does
> > not translate errnos Linux -> macOS, then client is broken. In the end it
> > matters what the protocol documentation specified.
> > 
> > Which client is that? Is it from Apple or from a third party? And are you
> > sure you were actually using 9p2000.L and not 9p2000.u?
> 
> I was asking this question because that's something that bothered us
> when developing the Windows patch because the errno translation does
> not exist in current macOS support, which means when using 9p2000.L:
> 
> * if testing a Linux 9 client with a macOS QEMU, it should have been
> noticed before due to the errno mismatch which this patch is trying to
> fix
> * the same error mismatch should happen between a macOS 9p client and
> a Linux QEMU

The 9p support patches for macOS hosts just landed few weeks ago on QEMU. And 
yes, this problem unfortunately slipped through unnoticed during review of the 
macOS patches, but will be addressed with upcoming QEMU stable bugfix release 
7.0.1.

Also note that most distributions don't pick the very first version of a new 
QEMU feature release cycle but rather wait for at least one or two stable 
revisions. So I don't expect that QEMU 7.0 is widely used on production 
systems yet, and it was just bad luck that your Windows patches appeared 
almost at the same time as the macOS patches. :/

> * or maybe only macOS 9p client was tested with a macOS QEMU, because
> no errno translation exist, which means the macOS 9p client is broken?

It does not help to repeat the same question and not answering other people's 
ones, so again:

Which 9p client is that exactly you are talking about? Is it an official 
client from Apple or rather an unofficial client from some third party source? 
You find quite a bunch of 9p client implementations out there. And are you 
sure you were actually using 9p2000.L and not 9p2000.u during your tests?

As for the macOS host patches: AFAIK nobody ever tested this with a macOS 
guest before. The original submitters (which you find on CC here) said they 
tested the macOS host patches with a Linux guest only, probably limited to 
very simple tasks.

The thing here is that some major errnos are identical on macOS and Linux. So 
if you are doing just simple stuff with 9p, then you can by luck get away 
without any errno translation. However at a certain point, when you do more 
fancy stuff, it will eventually hit you for sure.

Best regards,
Christian Schoenebeck

Re: [PATCH v5 2/6] 9pfs: fix qemu_mknodat(S_IFSOCK) on macOS

[Christian Schoenebeck]

On Freitag, 29. April 2022 16:35:07 CEST Greg Kurz wrote:
> On Fri, 29 Apr 2022 15:50:35 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > On Freitag, 29. April 2022 14:56:50 CEST Greg Kurz wrote:
> > > On Fri, 29 Apr 2022 12:25:11 +0200
> > > 
> > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > > mknod() on macOS does not support creating sockets, so divert to
> > > > call sequence socket(), bind() and fchmodat() respectively if S_IFSOCK
> > > > was passed with mode argument.
> > > > 
> > > > Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
> > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > > ---
> > > > 
> > > >  hw/9pfs/9p-util-darwin.c | 42
> > > >  +++++++++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 41 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> > > > index e24d09763a..619c403ba7 100644
> > > > --- a/hw/9pfs/9p-util-darwin.c
> > > > +++ b/hw/9pfs/9p-util-darwin.c
> > > > @@ -74,6 +74,42 @@ int fsetxattrat_nofollow(int dirfd, const char
> > > > *filename, const char *name,>
> > > > 
> > > >   */
> > > >  
> > > >  #if defined CONFIG_PTHREAD_FCHDIR_NP
> > > > 
> > > > +static int create_socket_file_at_cwd(const char *filename, mode_t
> > > > mode) {
> > > > +    int fd, err;
> > > > +    struct sockaddr_un addr = {
> > > > +        .sun_family = AF_UNIX
> > > > +    };
> > > > +
> > > > +    err = snprintf(addr.sun_path, sizeof(addr.sun_path), "./%s",
> > > > filename); +    if (err < 0 || err >= sizeof(addr.sun_path)) {
> > > 
> > > According to POSIX [1]:
> > > 
> > > The snprintf() function shall fail if:
> > > 
> > > [EOVERFLOW]
> > > [CX] [Option Start] The value of n is greater than {INT_MAX}. [Option
> > > End]
> > > 
> > > [1]
> > > https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.htm
> > > l
> > > 
> > > Since we're passing sizeof(addr.sun_path), I'm pretty sure snprintf()
> > > cannot fail. No big deal.
> > 
> > The question is whom you would want to trust on this? POSIX? ISO-C? Clang?
> > BSD? Apple? And for how long into future? I mean in general yes, I would
> > not
> To improve overall portability across all possible hosts, I'd stick to
> POSIX semantics but here this is macOS only code so you can assume
> this is Apple's snprintf().
> 
> > expect it to fail with -1 here either, but there are various different API
> > docs on snprintf() out there, and most of them don't even bother to
> > enumarate which encoding errors may happen. And I'm pretty sure if I'd
> > drop the negative err check here, then Akihiko would slap me for
> > unforeseeable additional error cases on snprintf() that may be added in
> > future.
> 
> /o\ ;-)
> 
> > Apple's documentation on snprintf() BTW just says:
> >   "These functions return a negative value if an error occurs."
> 
> How valuable this is !!! ;-)
> 
> > So Apple does not even restrict the return value to -1 on errrors, you
> > would also need to expect other negative values.
> > 
> > So on doubt, I leave this negative result check for now. ;-)
> 
> Fair enough.

Hey, don't shoot the servant! I'm just trying to find compromises that aim to 
suit as many people as possible, as always. :)

Best regards,
Christian Schoenebeck

Re: [PATCH v5 4/6] 9pfs: fix wrong errno being sent to Linux client on macOS host

[Bin Meng]

On Fri, Apr 29, 2022 at 11:16 PM Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
>
> On Freitag, 29. April 2022 16:16:54 CEST Bin Meng wrote:
> > On Fri, Apr 29, 2022 at 9:48 PM Christian Schoenebeck
> >
> > <qemu_oss@crudebyte.com> wrote:
> > > On Freitag, 29. April 2022 15:29:15 CEST Greg Kurz wrote:
> > > > On Fri, 29 Apr 2022 21:19:51 +0800
> > > >
> > > > Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > On Fri, Apr 29, 2022 at 9:08 PM Greg Kurz <groug@kaod.org> wrote:
> > > > > > On Fri, 29 Apr 2022 14:46:26 +0200
> > > > > >
> > > > > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > > > > > On Freitag, 29. April 2022 13:28:39 CEST Bin Meng wrote:
> > > > > > > > On Fri, Apr 29, 2022 at 7:16 PM 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 'Rlerror' replies (if 9p2000.L is used that
> > > > > > > > > is).
> > > > > > > > > Not
> > > > > > > > > having translated errnos before violated the 9p2000.L protocol
> > > > > > > > > spec,
> > > > > > > > >
> > > > > > > > > which says:
> > > > > > > > >   "
> > > > > > > > >   size[4] Rlerror tag[2] ecode[4]
> > > > > > > > >
> > > > > > > > >   ... ecode is a numerical Linux errno.
> > > > > > > > >   "
> > > > > > > > >
> > > > > > > > >   https://github.com/chaos/diod/wiki/protocol#lerror----return
> > > > > > > > >   -err
> > > > > > > > >   or-code
> > > > > > > > >
> > > > > > > > > This patch fixes a bunch of misbehaviours when running a Linux
> > > > > > > > > client
> > > > > > > > >
> > > > > > > > > on macOS host. 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.
> > > > > > > >
> > > > > > > > This issue looks exact the same issue we were trying to fix when
> > > > > > > > supporting 9p on Windows host,
> > > > > > > >
> > > > > > > > What we did is like this:
> > > > > > > > http://patchwork.ozlabs.org/project/qemu-devel/patch/20220425142
> > > > > > > > 705.
> > > > > > > > 2099270-> 10-bmeng.cn@gmail.com/
> > > > > > > >
> > > > > > > > But we had some questions in mind (see the commit message of our
> > > > > > > > patch, and below)
> > > > > > > >
> > > > > > > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > > > > > > > Link:
> > > > > > > > > https://lore.kernel.org/qemu-devel/20220421124835.3e664669@bah
> > > > > > > > > ia/
> > > > > > > > > Reviewed-by: Greg Kurz <groug@kaod.org>
> > > > > > > > > ---
> > > > > > > > >
> > > > > > > > >  hw/9pfs/9p-util.h | 30 ++++++++++++++++++++++++++++++
> > > > > > > > >  hw/9pfs/9p.c      |  2 ++
> > > > > > > > >  2 files changed, 32 insertions(+)
> > > > > > > > >
> > > > > > > > > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> > > > > > > > > index 2cc9a5dbfb..c3526144c9 100644
> > > > > > > > > --- a/hw/9pfs/9p-util.h
> > > > > > > > > +++ b/hw/9pfs/9p-util.h
> > > > > > > > > @@ -58,6 +58,36 @@ static inline uint64_t
> > > > > > > > > host_dev_to_dotl_dev(dev_t dev)
> > > > > > > > >
> > > > > > > > >  #endif
> > > > > > > > >  }
> > > > > > > > >
> > > > > > > > > +/* Translates errno from host -> Linux if needed */
> > > > > > > > > +static inline int errno_to_dotl(int err) {
> > > > > > > > > +#if defined(CONFIG_LINUX)
> > > > > > > > > +    /* nothing to translate (Linux -> Linux) */
> > > > > > > > > +#elif defined(CONFIG_DARWIN)
> > > > > > > > > +    /*
> > > > > > > > > +     * translation mandatory for macOS hosts
> > > > > > > > > +     *
> > > > > > > > > +     * FIXME: Only most important errnos translated here yet,
> > > > > > > > > this should
> > > > > > > > > be +     * extended to as many errnos being translated as
> > > > > > > > > possible
> > > > > > > > > in
> > > > > > > > > future. +     */
> > > > > > > > > +    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 */
> > > > > > > > > +    }
> > > > > > > >
> > > > > > > > What happens if a macOS guest is running on QEMU from a macOS
> > > > > > > > host?
> > > > > > > > Here all macOS errnos are translated to the Linux errnos. Will
> > > > > > > > macOS
> > > > > > > > be happy?
> > > > > > >
> > > > > > > Look at the commit log of this patch: it is a matter of which
> > > > > > > protocol
> > > > > > > is used> > >
> > > > > > >
> > > > > > > (currently there are 3 [1] protocol versions):
> > > > > > >    * 9p2000: nothing to translate here, as this protocol version
> > > > > > >    does
> > > > > > >    not
> > > > > > >
> > > > > > >      return a numeric error code, it only returns an error string
> > > > > > >      (and
> > > > > > >      we are
> > > > > > >      no longer supporting 9p2000 version in QEMU anyway BTW [1]):
> > > > > > >      http://ericvh.github.io/9p-rfc/rfc9p2000.html#anchor27
> > > > > > >
> > > > > > >    * 9p2000.L: errno *must* be in Linux errno mapping:
> > > > > > >      https://github.com/chaos/diod/wiki/protocol#lerror----return-> > > > > >      erro
> > > > > > >      r-code
> > > > > > >
> > > > > > >    * 9p2000.u: this one returns both an error code and error
> > > > > > >    string,
> > > > > > >    and it
> > > > > > >
> > > > > > >      says the error string should be preferred being interpreted
> > > > > > >      by
> > > > > > >      client:
> > > > > > >      http://ericvh.github.io/9p-rfc/rfc9p2000.u.html#anchor15
> > > > > > >
> > > > > > > In this patch here I only translated errno for 9p2000.L, whereas
> > > > > > > you
> > > > > > > are
> > > > > > > always translating it, no matter wich protocol version is used.
> > > > > > > You
> > > > > > > might
> > > > > > > argue that there should be a translation for 9p2000.u as well, but
> > > > > > > in
> > > > > > > the end we don't know the OS running on guest in this case. It
> > > > > > > could
> > > > > > > be Linux or something else.
> > > > > >
> > > > > > In the case of 9p2000.u the spec says "to provide a hint of the
> > > > > > underlying
> > > > > > UNIX error number which caused the error on the server" and even
> > > > > > mentions
> > > > > > "consistency problems of mapping error numbers betweeen different
> > > > > > versions
> > > > > > of UNIX"... this basically means that errno in 9p2000.u is undefined
> > > > > > since
> > > > > > it depends on the host. It is thus unusable unless the guest runs a
> > > > > > compatible UNIX variant. In any case, there's really nothing to
> > > > > > translate.
> > > > > >
> > > > > > > [1] https://wiki.qemu.org/Documentation/9p#9p_Protocol
> > > > >
> > > > > Thanks for the clarifications and pointers to different protocols! It
> > > > > looks what we did in our Windows patch is correct.
> > >
> > > Like I said, you are translating it for all protocol version, whereas this
> > > patch here translates errnos for 9p2000.L version only.
> >
> > Yes, this should be fixed in v2.
>
> Please wait for posting a v2 of those Windows patches. As you already
> realized, the macOS fix patches here and your Windows host support patches
> collide at several places.
>
> You can roughly expect these macOS patches to be merged on QEMU master branch
> approximately early next week. Once they landed on QEMU master you can rebase
> your patches on top of them and send a v2 accordingly subsequently.

Sure.

>
> > > > > I have another question, does this mean the current 9pfs client on
> > > > > macOS is broken since it does not use any translation? With this
> > > > > patch, now the 9p server returns the translated linux errno so the 9p
> > > > > client on macOS should complain.
> > > >
> > > > I don't now the macOS client but if it doesn't expect linux errnos
> > > > then it is infringing 9p2000.L and should be fixed.
> > >
> > > Agreed, if you are using 9p2000.L with that macOS client and client does
> > > not translate errnos Linux -> macOS, then client is broken. In the end it
> > > matters what the protocol documentation specified.
> > >
> > > Which client is that? Is it from Apple or from a third party? And are you
> > > sure you were actually using 9p2000.L and not 9p2000.u?
> >
> > I was asking this question because that's something that bothered us
> > when developing the Windows patch because the errno translation does
> > not exist in current macOS support, which means when using 9p2000.L:
> >
> > * if testing a Linux 9 client with a macOS QEMU, it should have been
> > noticed before due to the errno mismatch which this patch is trying to
> > fix
> > * the same error mismatch should happen between a macOS 9p client and
> > a Linux QEMU
>
> The 9p support patches for macOS hosts just landed few weeks ago on QEMU. And
> yes, this problem unfortunately slipped through unnoticed during review of the
> macOS patches, but will be addressed with upcoming QEMU stable bugfix release
> 7.0.1.
>
> Also note that most distributions don't pick the very first version of a new
> QEMU feature release cycle but rather wait for at least one or two stable
> revisions. So I don't expect that QEMU 7.0 is widely used on production
> systems yet, and it was just bad luck that your Windows patches appeared
> almost at the same time as the macOS patches. :/

Good to know.

>
> > * or maybe only macOS 9p client was tested with a macOS QEMU, because
> > no errno translation exist, which means the macOS 9p client is broken?
>
> It does not help to repeat the same question and not answering other people's
> ones, so again:
>
> Which 9p client is that exactly you are talking about? Is it an official
> client from Apple or rather an unofficial client from some third party source?
> You find quite a bunch of 9p client implementations out there. And are you
> sure you were actually using 9p2000.L and not 9p2000.u during your tests?

I think I already answered your question, and the answer is that we
just don't know. Hence why I was asking questions in this thread. If
you examine the commit message of the Windows patch, we described our
question or confusion about macOS support that we suspected macOS
support is broken with regard to the errno, but we cannot confirm it
because we don't have macOS to test.

>
> As for the macOS host patches: AFAIK nobody ever tested this with a macOS
> guest before. The original submitters (which you find on CC here) said they
> tested the macOS host patches with a Linux guest only, probably limited to
> very simple tasks.

Ah, this explains. Thanks!

> The thing here is that some major errnos are identical on macOS and Linux. So
> if you are doing just simple stuff with 9p, then you can by luck get away
> without any errno translation. However at a certain point, when you do more
> fancy stuff, it will eventually hit you for sure.

Yes, I think I get the whole picture now :)

Regards,
Bin

Re: [PATCH v5 2/6] 9pfs: fix qemu_mknodat(S_IFSOCK) on macOS

[Akihiko Odaki]

On 2022/04/30 0:20, Christian Schoenebeck wrote:
> On Freitag, 29. April 2022 16:35:07 CEST Greg Kurz wrote:
>> On Fri, 29 Apr 2022 15:50:35 +0200
>>
>> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
>>> On Freitag, 29. April 2022 14:56:50 CEST Greg Kurz wrote:
>>>> On Fri, 29 Apr 2022 12:25:11 +0200
>>>>
>>>> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
>>>>> mknod() on macOS does not support creating sockets, so divert to
>>>>> call sequence socket(), bind() and fchmodat() respectively if S_IFSOCK
>>>>> was passed with mode argument.
>>>>>
>>>>> Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
>>>>> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
>>>>> ---
>>>>>
>>>>>   hw/9pfs/9p-util-darwin.c | 42
>>>>>   +++++++++++++++++++++++++++++++++++++++-
>>>>>   1 file changed, 41 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
>>>>> index e24d09763a..619c403ba7 100644
>>>>> --- a/hw/9pfs/9p-util-darwin.c
>>>>> +++ b/hw/9pfs/9p-util-darwin.c
>>>>> @@ -74,6 +74,42 @@ int fsetxattrat_nofollow(int dirfd, const char
>>>>> *filename, const char *name,>
>>>>>
>>>>>    */
>>>>>   
>>>>>   #if defined CONFIG_PTHREAD_FCHDIR_NP
>>>>>
>>>>> +static int create_socket_file_at_cwd(const char *filename, mode_t
>>>>> mode) {
>>>>> +    int fd, err;
>>>>> +    struct sockaddr_un addr = {
>>>>> +        .sun_family = AF_UNIX
>>>>> +    };
>>>>> +
>>>>> +    err = snprintf(addr.sun_path, sizeof(addr.sun_path), "./%s",
>>>>> filename); +    if (err < 0 || err >= sizeof(addr.sun_path)) {
>>>>
>>>> According to POSIX [1]:
>>>>
>>>> The snprintf() function shall fail if:
>>>>
>>>> [EOVERFLOW]
>>>> [CX] [Option Start] The value of n is greater than {INT_MAX}. [Option
>>>> End]
>>>>
>>>> [1]
>>>> https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.htm
>>>> l
>>>>
>>>> Since we're passing sizeof(addr.sun_path), I'm pretty sure snprintf()
>>>> cannot fail. No big deal.
>>>
>>> The question is whom you would want to trust on this? POSIX? ISO-C? Clang?
>>> BSD? Apple? And for how long into future? I mean in general yes, I would
>>> not
>> To improve overall portability across all possible hosts, I'd stick to
>> POSIX semantics but here this is macOS only code so you can assume
>> this is Apple's snprintf().
>>
>>> expect it to fail with -1 here either, but there are various different API
>>> docs on snprintf() out there, and most of them don't even bother to
>>> enumarate which encoding errors may happen. And I'm pretty sure if I'd
>>> drop the negative err check here, then Akihiko would slap me for
>>> unforeseeable additional error cases on snprintf() that may be added in
>>> future. >>
>> /o\ ;-)

I would rather throw the system which decided returning a negative value 
for snprintf() OK away, but it is always good to be cautious anyway. ;)

For the entire series:
Reviewed-by: Akihiko Odaki <akihiko.odaki@gmail.com>

>>
>>> Apple's documentation on snprintf() BTW just says:
>>>    "These functions return a negative value if an error occurs."
>>
>> How valuable this is !!! ;-)
>>
>>> So Apple does not even restrict the return value to -1 on errrors, you
>>> would also need to expect other negative values.
>>>
>>> So on doubt, I leave this negative result check for now. ;-)
>>
>> Fair enough.
> 
> Hey, don't shoot the servant! I'm just trying to find compromises that aim to
> suit as many people as possible, as always. :)
> 
> Best regards,
> Christian Schoenebeck
> 
> 

Re: [PATCH v5 0/6] 9pfs: macOS host fixes (resend)

[Christian Schoenebeck]

On Freitag, 29. April 2022 12:26:40 CEST Christian Schoenebeck wrote:
> A bunch of fixes for recently (in QEMU 7.0) added 9p support on macOS hosts.
> 
> Note: there are still issues to address with case-insensitive file systems
> on macOS hosts. I sent a separate RFC on that icase issue:
> https://lore.kernel.org/qemu-devel/1757498.AyhHxzoH2B@silver/
> 
> v4 -> v5:
> 
>   * Check return value of snprintf() instead of strlen(filename).
>     [patch 2]
> 
> Christian Schoenebeck (6):
>   9pfs: fix qemu_mknodat(S_IFREG) on macOS
>   9pfs: fix qemu_mknodat(S_IFSOCK) on macOS
>   9pfs: fix wrong encoding of rdev field in Rgetattr on macOS
>   9pfs: fix wrong errno being sent to Linux client on macOS host
>   9pfs: fix removing non-existent POSIX ACL xattr on macOS host
>   9pfs: fix qemu_mknodat() to always return -1 on error on macOS host
> 
>  hw/9pfs/9p-posix-acl.c   | 12 +++++--
>  hw/9pfs/9p-util-darwin.c | 54 +++++++++++++++++++++++++++++--
>  hw/9pfs/9p-util.h        | 69 ++++++++++++++++++++++++++++++++++++++++
>  hw/9pfs/9p.c             |  4 ++-
>  4 files changed, 134 insertions(+), 5 deletions(-)

Queued on 9p.next:
https://github.com/cschoenebeck/qemu/commits/9p.next

Thanks!

I just sent a PR:
https://lore.kernel.org/all/cover.1651319081.git.qemu_oss@crudebyte.com/

Best regards,
Christian Schoenebeck


[Resent this message due to mail delivery error from nongnu.org lists]

Re: [PATCH v5 2/6] 9pfs: fix qemu_mknodat(S_IFSOCK) on macOS

[Greg Kurz]

On Fri, 29 Apr 2022 17:20:26 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Freitag, 29. April 2022 16:35:07 CEST Greg Kurz wrote:
> > On Fri, 29 Apr 2022 15:50:35 +0200
> > 
> > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > On Freitag, 29. April 2022 14:56:50 CEST Greg Kurz wrote:
> > > > On Fri, 29 Apr 2022 12:25:11 +0200
> > > > 
> > > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > > > mknod() on macOS does not support creating sockets, so divert to
> > > > > call sequence socket(), bind() and fchmodat() respectively if S_IFSOCK
> > > > > was passed with mode argument.
> > > > > 
> > > > > Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
> > > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > > > ---
> > > > > 
> > > > >  hw/9pfs/9p-util-darwin.c | 42
> > > > >  +++++++++++++++++++++++++++++++++++++++-
> > > > >  1 file changed, 41 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> > > > > index e24d09763a..619c403ba7 100644
> > > > > --- a/hw/9pfs/9p-util-darwin.c
> > > > > +++ b/hw/9pfs/9p-util-darwin.c
> > > > > @@ -74,6 +74,42 @@ int fsetxattrat_nofollow(int dirfd, const char
> > > > > *filename, const char *name,>
> > > > > 
> > > > >   */
> > > > >  
> > > > >  #if defined CONFIG_PTHREAD_FCHDIR_NP
> > > > > 
> > > > > +static int create_socket_file_at_cwd(const char *filename, mode_t
> > > > > mode) {
> > > > > +    int fd, err;
> > > > > +    struct sockaddr_un addr = {
> > > > > +        .sun_family = AF_UNIX
> > > > > +    };
> > > > > +
> > > > > +    err = snprintf(addr.sun_path, sizeof(addr.sun_path), "./%s",
> > > > > filename); +    if (err < 0 || err >= sizeof(addr.sun_path)) {
> > > > 
> > > > According to POSIX [1]:
> > > > 
> > > > The snprintf() function shall fail if:
> > > > 
> > > > [EOVERFLOW]
> > > > [CX] [Option Start] The value of n is greater than {INT_MAX}. [Option
> > > > End]
> > > > 
> > > > [1]
> > > > https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.htm
> > > > l
> > > > 
> > > > Since we're passing sizeof(addr.sun_path), I'm pretty sure snprintf()
> > > > cannot fail. No big deal.
> > > 
> > > The question is whom you would want to trust on this? POSIX? ISO-C? Clang?
> > > BSD? Apple? And for how long into future? I mean in general yes, I would
> > > not
> > To improve overall portability across all possible hosts, I'd stick to
> > POSIX semantics but here this is macOS only code so you can assume
> > this is Apple's snprintf().
> > 
> > > expect it to fail with -1 here either, but there are various different API
> > > docs on snprintf() out there, and most of them don't even bother to
> > > enumarate which encoding errors may happen. And I'm pretty sure if I'd
> > > drop the negative err check here, then Akihiko would slap me for
> > > unforeseeable additional error cases on snprintf() that may be added in
> > > future.
> > 
> > /o\ ;-)
> > 
> > > Apple's documentation on snprintf() BTW just says:
> > >   "These functions return a negative value if an error occurs."
> > 
> > How valuable this is !!! ;-)
> > 
> > > So Apple does not even restrict the return value to -1 on errrors, you
> > > would also need to expect other negative values.
> > > 
> > > So on doubt, I leave this negative result check for now. ;-)
> > 
> > Fair enough.
> 
> Hey, don't shoot the servant! I'm just trying to find compromises that aim to 
> suit as many people as possible, as always. :)
> 

Oh I wasn't criticizing your work... rather feeling sorry for the
poor documentation of snprintf() on the Apple side. Please go
on with the great job you're doing on 9p ! :-)

Cheers,

--
Greg

> Best regards,
> Christian Schoenebeck
> 
>