qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] 9pfs: Add FreeBSD support
@ 2025-08-06 17:53 Mark Johnston
  2025-08-21 11:24 ` Christian Schoenebeck
  2025-09-05 11:19 ` Christian Schoenebeck
  0 siblings, 2 replies; 8+ messages in thread
From: Mark Johnston @ 2025-08-06 17:53 UTC (permalink / raw)
  To: qemu-devel
  Cc: Christian Schoenebeck, Greg Kurz, Paolo Bonzini,
	Marc-André Lureau, Daniel P. Berrangé,
	Philippe Mathieu-Daudé

This is largely derived from existing Darwin support.  FreeBSD
apparently has better support for *at() system calls so doesn't require
workarounds for a missing mknodat().  The implementation has a couple of
warts however:
- The extattr(2) system calls don't support anything akin to
  XATTR_CREATE or XATTR_REPLACE, so a racy workaround is implemented.
- Attribute names cannot begin with "user." on ZFS, so the prefix is
  trimmed off.  FreeBSD's extattr system calls sport an extra
  "namespace" identifier, and attributes created by the 9pfs backend
  live in the universal user namespace, so this seems innocent enough.

The 9pfs tests were verified to pass on the UFS, ZFS and tmpfs
filesystems.

Signed-off-by: Mark Johnston <markj@FreeBSD.org>
---
Changes since v1:
- Handle extended attributes in the system.* namespace as well.
- Fix a typo in a comment.
- Clarify a comment.

 fsdev/file-op-9p.h        |   6 +-
 fsdev/meson.build         |   2 +-
 hw/9pfs/9p-synth.c        |   2 +-
 hw/9pfs/9p-util-freebsd.c | 132 ++++++++++++++++++++++++++++++++++++++
 hw/9pfs/9p-util.h         |  20 ++++--
 hw/9pfs/9p.c              |  16 ++++-
 hw/9pfs/meson.build       |   2 +
 include/qemu/xattr.h      |   6 +-
 meson.build               |   8 +--
 9 files changed, 179 insertions(+), 15 deletions(-)
 create mode 100644 hw/9pfs/9p-util-freebsd.c

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index b9dae8c84c..b85c9934de 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -21,9 +21,11 @@
 
 #ifdef CONFIG_LINUX
 # include <sys/vfs.h>
-#endif
-#ifdef CONFIG_DARWIN
+#elif defined(CONFIG_DARWIN) || defined(CONFIG_FREEBSD)
 # include <sys/param.h>
+# ifdef CONFIG_FREEBSD
+#  undef MACHINE /* work around some unfortunate namespace pollution */
+# endif
 # include <sys/mount.h>
 #endif
 
diff --git a/fsdev/meson.build b/fsdev/meson.build
index c751d8cb62..95fe816604 100644
--- a/fsdev/meson.build
+++ b/fsdev/meson.build
@@ -5,6 +5,6 @@ fsdev_ss.add(when: ['CONFIG_FSDEV_9P'], if_true: files(
   '9p-marshal.c',
   'qemu-fsdev.c',
 ), if_false: files('qemu-fsdev-dummy.c'))
-if host_os in ['linux', 'darwin']
+if host_os in ['linux', 'darwin', 'freebsd']
   system_ss.add_all(fsdev_ss)
 endif
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index 9cd1884224..b3743f6169 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -451,7 +451,7 @@ static int synth_statfs(FsContext *s, V9fsPath *fs_path,
     stbuf->f_bsize = 512;
     stbuf->f_blocks = 0;
     stbuf->f_files = synth_node_count;
-#ifndef CONFIG_DARWIN
+#if !defined(CONFIG_DARWIN) && !defined(CONFIG_FREEBSD)
     stbuf->f_namelen = NAME_MAX;
 #endif
     return 0;
diff --git a/hw/9pfs/9p-util-freebsd.c b/hw/9pfs/9p-util-freebsd.c
new file mode 100644
index 0000000000..9dd1d069f6
--- /dev/null
+++ b/hw/9pfs/9p-util-freebsd.c
@@ -0,0 +1,132 @@
+/*
+ * 9p utilities (FreeBSD Implementation)
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/xattr.h"
+#include "9p-util.h"
+
+static int mangle_xattr_name(const char **namep)
+{
+    const char *name = *namep;
+
+    /*
+     * ZFS forbids attributes in starting with "user." or "system.".
+     */
+    if (strncmp(name, "system.", 7) == 0) {
+        *namep = name + 7;
+        return EXTATTR_NAMESPACE_SYSTEM;
+    }
+    if (strncmp(name, "user.", 5) == 0) {
+        *namep = name + 5;
+    }
+    return EXTATTR_NAMESPACE_USER;
+}
+
+ssize_t fgetxattr(int fd, const char *name, void *value, size_t size)
+{
+    int namespace;
+
+    namespace = mangle_xattr_name(&name);
+    return extattr_get_fd(fd, namespace, name, value, size);
+}
+
+ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char *name,
+                             void *value, size_t size)
+{
+    ssize_t ret;
+    int fd, namespace;
+
+    fd = openat_file(dirfd, filename,
+                     O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+    if (fd == -1) {
+        return -1;
+    }
+    namespace = mangle_xattr_name(&name);
+    ret = extattr_get_fd(fd, namespace, name, value, size);
+    close_preserve_errno(fd);
+    return ret;
+}
+
+ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
+                              char *list, size_t size)
+{
+    ssize_t ret;
+    int fd;
+
+    fd = openat_file(dirfd, filename,
+                     O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+    if (fd == -1) {
+        return -1;
+    }
+    ret = extattr_list_fd(fd, EXTATTR_NAMESPACE_USER, list, size);
+    close_preserve_errno(fd);
+    return ret;
+}
+
+ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
+                                const char *name)
+{
+    int fd, namespace, ret;
+
+    fd = openat_file(dirfd, filename,
+                     O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+    if (fd == -1) {
+        return -1;
+    }
+    namespace = mangle_xattr_name(&name);
+    ret = extattr_delete_fd(fd, namespace, name);
+    close_preserve_errno(fd);
+    return ret;
+}
+
+int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
+                         void *value, size_t size, int flags)
+{
+    ssize_t ret;
+    int fd, namespace;
+
+    namespace = mangle_xattr_name(&name);
+    if (flags == (XATTR_CREATE | XATTR_REPLACE)) {
+        errno = EINVAL;
+        return -1;
+    }
+    fd = openat_file(dirfd, filename,
+                     O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+    if (fd == -1) {
+        return -1;
+    }
+    if (flags & (XATTR_CREATE | XATTR_REPLACE)) {
+        ret = extattr_get_fd(fd, namespace, name, NULL, 0);
+        if (ret == -1 && errno != ENOATTR) {
+            close_preserve_errno(fd);
+            return -1;
+        }
+        if (ret >= 0 && (flags & XATTR_CREATE)) {
+            errno = EEXIST;
+            close_preserve_errno(fd);
+            return -1;
+        }
+        if (ret == -1 && (flags & XATTR_REPLACE)) {
+            errno = ENOATTR;
+            close_preserve_errno(fd);
+            return -1;
+        }
+    }
+    ret = extattr_set_fd(fd, namespace, name, value, size);
+    close_preserve_errno(fd);
+    return ret;
+}
+
+int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
+{
+    return mknodat(dirfd, filename, mode, dev);
+}
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index a1924fe3f0..8dfa803dc2 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -21,6 +21,15 @@
 #define O_PATH_9P_UTIL 0
 #endif
 
+#ifdef CONFIG_FREEBSD
+/*
+ * FreeBSD does not have these flags, so we can only emulate their intended
+ * behaviour (racily).
+ */
+#define XATTR_CREATE    0x1
+#define XATTR_REPLACE   0x2
+#endif
+
 #if !defined(CONFIG_LINUX)
 
 /*
@@ -64,9 +73,9 @@ static inline uint64_t host_dev_to_dotl_dev(dev_t dev)
 static inline int errno_to_dotl(int err) {
 #if defined(CONFIG_LINUX)
     /* nothing to translate (Linux -> Linux) */
-#elif defined(CONFIG_DARWIN)
+#elif defined(CONFIG_DARWIN) || defined(CONFIG_FREEBSD)
     /*
-     * translation mandatory for macOS hosts
+     * translation mandatory for non-Linux hosts
      *
      * FIXME: Only most important errnos translated here yet, this should be
      * extended to as many errnos being translated as possible in future.
@@ -155,13 +164,13 @@ static inline int openat_file(int dirfd, const char *name, int flags,
 {
     int fd, serrno, ret;
 
-#ifndef CONFIG_DARWIN
+#if !defined(CONFIG_DARWIN) && !defined(CONFIG_FREEBSD)
 again:
 #endif
     fd = qemu_openat(dirfd, name, flags | O_NOFOLLOW | O_NOCTTY | O_NONBLOCK,
                      mode);
     if (fd == -1) {
-#ifndef CONFIG_DARWIN
+#if !defined(CONFIG_DARWIN) && !defined(CONFIG_FREEBSD)
         if (errno == EPERM && (flags & O_NOATIME)) {
             /*
              * The client passed O_NOATIME but we lack permissions to honor it.
@@ -202,6 +211,9 @@ again:
     return fd;
 }
 
+#ifdef CONFIG_FREEBSD
+ssize_t fgetxattr(int dirfd, const char *name, void *value, size_t size);
+#endif
 ssize_t fgetxattrat_nofollow(int dirfd, const char *path, const char *name,
                              void *value, size_t size);
 int fsetxattrat_nofollow(int dirfd, const char *path, const char *name,
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index acfa7db4e1..bc4a016ee3 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -136,8 +136,10 @@ static int dotl_to_open_flags(int flags)
         { P9_DOTL_NONBLOCK, O_NONBLOCK } ,
         { P9_DOTL_DSYNC, O_DSYNC },
         { P9_DOTL_FASYNC, FASYNC },
-#ifndef CONFIG_DARWIN
+#if !defined(CONFIG_DARWIN) && !defined(CONFIG_FREEBSD)
         { P9_DOTL_NOATIME, O_NOATIME },
+#endif
+#ifndef CONFIG_DARWIN
         /*
          *  On Darwin, we could map to F_NOCACHE, which is
          *  similar, but doesn't quite have the same
@@ -3658,7 +3660,7 @@ static int v9fs_fill_statfs(V9fsState *s, V9fsPDU *pdu, struct statfs *stbuf)
     f_bavail = stbuf->f_bavail / bsize_factor;
     f_files  = stbuf->f_files;
     f_ffree  = stbuf->f_ffree;
-#ifdef CONFIG_DARWIN
+#if defined(CONFIG_DARWIN) || defined(CONFIG_FREEBSD)
     fsid_val = (unsigned int)stbuf->f_fsid.val[0] |
                (unsigned long long)stbuf->f_fsid.val[1] << 32;
     f_namelen = NAME_MAX;
@@ -4050,6 +4052,16 @@ out_nofid:
  * Linux guests.
  */
 #define P9_XATTR_SIZE_MAX 65536
+#elif defined(CONFIG_FREEBSD)
+/*
+ * FreeBSD similarly doesn't define a maximum xattr size, the limit is
+ * filesystem dependent.  On UFS filesystems it's 2 times the filesystem block
+ * size, typically 32KB.  On ZFS it depends on the value of the xattr property;
+ * with the default value there is no limit, and with xattr=sa it is 64KB.
+ *
+ * So, a limit of 64k seems reasonable here too.
+ */
+#define P9_XATTR_SIZE_MAX 65536
 #else
 #error Missing definition for P9_XATTR_SIZE_MAX for this host system
 #endif
diff --git a/hw/9pfs/meson.build b/hw/9pfs/meson.build
index d35d4f44ff..7f4d6e3a45 100644
--- a/hw/9pfs/meson.build
+++ b/hw/9pfs/meson.build
@@ -15,6 +15,8 @@ fs_ss.add(files(
 ))
 if host_os == 'darwin'
   fs_ss.add(files('9p-util-darwin.c'))
+elif host_os == 'freebsd'
+  fs_ss.add(files('9p-util-freebsd.c'))
 elif host_os == 'linux'
   fs_ss.add(files('9p-util-linux.c'))
 endif
diff --git a/include/qemu/xattr.h b/include/qemu/xattr.h
index b08a934acc..224ba1276e 100644
--- a/include/qemu/xattr.h
+++ b/include/qemu/xattr.h
@@ -26,7 +26,11 @@
 #    define ENOATTR ENODATA
 #  endif
 #  ifndef CONFIG_WIN32
-#    include <sys/xattr.h>
+#    ifdef CONFIG_FREEBSD
+#      include <sys/extattr.h>
+#    else
+#      include <sys/xattr.h>
+#    endif
 #  endif
 #endif
 
diff --git a/meson.build b/meson.build
index c2bc3eeedc..4c681bd2e8 100644
--- a/meson.build
+++ b/meson.build
@@ -2382,11 +2382,11 @@ dbus_display = get_option('dbus_display') \
   .allowed()
 
 have_virtfs = get_option('virtfs') \
-    .require(host_os == 'linux' or host_os == 'darwin',
-             error_message: 'virtio-9p (virtfs) requires Linux or macOS') \
-    .require(host_os == 'linux' or cc.has_function('pthread_fchdir_np'),
+    .require(host_os == 'linux' or host_os == 'darwin' or host_os == 'freebsd',
+             error_message: 'virtio-9p (virtfs) requires Linux or macOS or FreeBSD') \
+    .require(host_os != 'darwin' or cc.has_function('pthread_fchdir_np'),
              error_message: 'virtio-9p (virtfs) on macOS requires the presence of pthread_fchdir_np') \
-    .require(host_os == 'darwin' or libattr.found(),
+    .require(host_os != 'linux' or libattr.found(),
              error_message: 'virtio-9p (virtfs) on Linux requires libattr-devel') \
     .disable_auto_if(not have_tools and not have_system) \
     .allowed()
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] 9pfs: Add FreeBSD support
  2025-08-06 17:53 [PATCH v2] 9pfs: Add FreeBSD support Mark Johnston
@ 2025-08-21 11:24 ` Christian Schoenebeck
  2025-08-25 12:53   ` Mark Johnston
  2025-09-05 11:19 ` Christian Schoenebeck
  1 sibling, 1 reply; 8+ messages in thread
From: Christian Schoenebeck @ 2025-08-21 11:24 UTC (permalink / raw)
  To: qemu-devel, Mark Johnston
  Cc: Greg Kurz, Paolo Bonzini, Marc-André Lureau,
	Daniel P. Berrangé, Philippe Mathieu-Daudé

On Wednesday, August 6, 2025 7:53:08 PM CEST Mark Johnston wrote:
> This is largely derived from existing Darwin support.  FreeBSD
> apparently has better support for *at() system calls so doesn't require
> workarounds for a missing mknodat().  The implementation has a couple of
> warts however:
> - The extattr(2) system calls don't support anything akin to
>   XATTR_CREATE or XATTR_REPLACE, so a racy workaround is implemented.
> - Attribute names cannot begin with "user." on ZFS, so the prefix is
>   trimmed off.  FreeBSD's extattr system calls sport an extra
>   "namespace" identifier, and attributes created by the 9pfs backend
>   live in the universal user namespace, so this seems innocent enough.
> 
> The 9pfs tests were verified to pass on the UFS, ZFS and tmpfs
> filesystems.
> 
> Signed-off-by: Mark Johnston <markj@FreeBSD.org>
> ---
> Changes since v1:
> - Handle extended attributes in the system.* namespace as well.
> - Fix a typo in a comment.
> - Clarify a comment.

Not forgotten. I just hoped there were other reviewers or testers in the 
meantime, but be it.

Like I said, I don't have FreeBSD system here to test this, so I am taking 
your word for now that you tested this and plan to bring this into QEMU when 
master re-opens for new features soon.

If you have some time to adjust the commit log message above, that would be 
great, otherwise I can also handle this on my end later on. Looks like that 
comment is not adjusted for v2 yet (i.e. "user." and not mentioning 
"system.").

/Christian




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] 9pfs: Add FreeBSD support
  2025-08-21 11:24 ` Christian Schoenebeck
@ 2025-08-25 12:53   ` Mark Johnston
  2025-08-26 12:34     ` Christian Schoenebeck
  0 siblings, 1 reply; 8+ messages in thread
From: Mark Johnston @ 2025-08-25 12:53 UTC (permalink / raw)
  To: Christian Schoenebeck
  Cc: qemu-devel, Greg Kurz, Paolo Bonzini, Marc-André Lureau,
	Daniel P. Berrangé, Philippe Mathieu-Daudé

On Thu, Aug 21, 2025 at 01:24:04PM +0200, Christian Schoenebeck wrote:
> On Wednesday, August 6, 2025 7:53:08 PM CEST Mark Johnston wrote:
> > This is largely derived from existing Darwin support.  FreeBSD
> > apparently has better support for *at() system calls so doesn't require
> > workarounds for a missing mknodat().  The implementation has a couple of
> > warts however:
> > - The extattr(2) system calls don't support anything akin to
> >   XATTR_CREATE or XATTR_REPLACE, so a racy workaround is implemented.
> > - Attribute names cannot begin with "user." on ZFS, so the prefix is
> >   trimmed off.  FreeBSD's extattr system calls sport an extra
> >   "namespace" identifier, and attributes created by the 9pfs backend
> >   live in the universal user namespace, so this seems innocent enough.
> > 
> > The 9pfs tests were verified to pass on the UFS, ZFS and tmpfs
> > filesystems.
> > 
> > Signed-off-by: Mark Johnston <markj@FreeBSD.org>
> > ---
> > Changes since v1:
> > - Handle extended attributes in the system.* namespace as well.
> > - Fix a typo in a comment.
> > - Clarify a comment.
> 
> Not forgotten. I just hoped there were other reviewers or testers in the 
> meantime, but be it.
> 
> Like I said, I don't have FreeBSD system here to test this, so I am taking 
> your word for now that you tested this and plan to bring this into QEMU when 
> master re-opens for new features soon.

Thank you very much!

In case I missed somewhat, what testing would you typically do
otherwise?  So far I had run the QEMU test suite (which indeed found
some bugs in the initial version) and tried mounting a 9pfs share from
Linux and FreeBSD guests and doing a bit of manual testing.

> If you have some time to adjust the commit log message above, that would be 
> great, otherwise I can also handle this on my end later on. Looks like that 
> comment is not adjusted for v2 yet (i.e. "user." and not mentioning 
> "system.").

Here's an amended commit log message.  Please let me know if this is
better submitted as a v3.

commit b79bf1b7d42025e3e14da86a7c08d269038cd3ed
Author: Mark Johnston <markj@FreeBSD.org>
Date:   Wed Jul 16 20:32:05 2025 +0000

    9pfs: Add FreeBSD support
    
    This is largely derived from existing Darwin support.  FreeBSD
    apparently has better support for *at() system calls so doesn't require
    workarounds for a missing mknodat().  The implementation has a couple of
    warts however:
    - The extattr(2) system calls don't support anything akin to
      XATTR_CREATE or XATTR_REPLACE, so a racy workaround is implemented.
    - Attribute names cannot begin with "user." or "system." on ZFS, so
      these prefixes are trimmed off.  FreeBSD's extattr system calls sport
      an extra "namespace" identifier, and attributes created by the 9pfs
      backend live in the universal user and system namespaces, so this
      seems innocent enough.
    
    The 9pfs tests were verified to pass on the UFS, ZFS and tmpfs
    filesystems.
    
    Signed-off-by: Mark Johnston <markj@FreeBSD.org>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] 9pfs: Add FreeBSD support
  2025-08-25 12:53   ` Mark Johnston
@ 2025-08-26 12:34     ` Christian Schoenebeck
  2025-08-27 14:29       ` Mark Johnston
  2025-09-01 10:40       ` Greg Kurz
  0 siblings, 2 replies; 8+ messages in thread
From: Christian Schoenebeck @ 2025-08-26 12:34 UTC (permalink / raw)
  To: qemu-devel, Mark Johnston
  Cc: Greg Kurz, Paolo Bonzini, Marc-André Lureau,
	Daniel P. Berrangé, Philippe Mathieu-Daudé

On Monday, August 25, 2025 2:53:06 PM CEST Mark Johnston wrote:
> On Thu, Aug 21, 2025 at 01:24:04PM +0200, Christian Schoenebeck wrote:
> > On Wednesday, August 6, 2025 7:53:08 PM CEST Mark Johnston wrote:
[...]
> > Not forgotten. I just hoped there were other reviewers or testers in the
> > meantime, but be it.
> > 
> > Like I said, I don't have FreeBSD system here to test this, so I am taking
> > your word for now that you tested this and plan to bring this into QEMU
> > when master re-opens for new features soon.
> 
> Thank you very much!
> 
> In case I missed somewhat, what testing would you typically do
> otherwise?  So far I had run the QEMU test suite (which indeed found
> some bugs in the initial version) and tried mounting a 9pfs share from
> Linux and FreeBSD guests and doing a bit of manual testing.

Apart from QEMU's test cases, I also use guest systems running 9p as root file 
system [1], run software compilations there among some things. That proofed to 
be quite a useful test environment to spot edge cases, concurrency and 
performance issues and such.

[1] https://wiki.qemu.org/Documentation/9p_root_fs

Greg was running some general purpose file system stress test suite in the 
past, but I currently can't recall what that was.

> > If you have some time to adjust the commit log message above, that would
> > be
> > great, otherwise I can also handle this on my end later on. Looks like
> > that
> > comment is not adjusted for v2 yet (i.e. "user." and not mentioning
> > "system.").
> 
> Here's an amended commit log message.  Please let me know if this is
> better submitted as a v3.
> 
> commit b79bf1b7d42025e3e14da86a7c08d269038cd3ed
> Author: Mark Johnston <markj@FreeBSD.org>
> Date:   Wed Jul 16 20:32:05 2025 +0000
> 
>     9pfs: Add FreeBSD support
> 
>     This is largely derived from existing Darwin support.  FreeBSD
>     apparently has better support for *at() system calls so doesn't require
>     workarounds for a missing mknodat().  The implementation has a couple of
> warts however:
>     - The extattr(2) system calls don't support anything akin to
>       XATTR_CREATE or XATTR_REPLACE, so a racy workaround is implemented.
>     - Attribute names cannot begin with "user." or "system." on ZFS, so
>       these prefixes are trimmed off.  FreeBSD's extattr system calls sport
>       an extra "namespace" identifier, and attributes created by the 9pfs
>       backend live in the universal user and system namespaces, so this
>       seems innocent enough.
> 
>     The 9pfs tests were verified to pass on the UFS, ZFS and tmpfs
>     filesystems.
> 
>     Signed-off-by: Mark Johnston <markj@FreeBSD.org>

Almost. Maybe something like this to make it a bit more clear?

- Attribute names cannot begin with "user." or "system." on ZFS. However
  FreeBSD's extattr(2) system supports two dedicated namespaces for these
  two. So "user." or "system." prefixes are trimmed off from attribute
  names and instead EXTATTR_NAMESPACE_USER or EXTATTR_NAMESPACE_SYSTEM
  are picked and passed to extattr system calls instead accordingly.

/Christian




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] 9pfs: Add FreeBSD support
  2025-08-26 12:34     ` Christian Schoenebeck
@ 2025-08-27 14:29       ` Mark Johnston
  2025-09-01 10:31         ` Christian Schoenebeck
  2025-09-01 10:40       ` Greg Kurz
  1 sibling, 1 reply; 8+ messages in thread
From: Mark Johnston @ 2025-08-27 14:29 UTC (permalink / raw)
  To: Christian Schoenebeck
  Cc: qemu-devel, Greg Kurz, Paolo Bonzini, Marc-André Lureau,
	Daniel P. Berrangé, Philippe Mathieu-Daudé

On Tue, Aug 26, 2025 at 02:34:45PM +0200, Christian Schoenebeck wrote:
> On Monday, August 25, 2025 2:53:06 PM CEST Mark Johnston wrote:
> > On Thu, Aug 21, 2025 at 01:24:04PM +0200, Christian Schoenebeck wrote:
> > > On Wednesday, August 6, 2025 7:53:08 PM CEST Mark Johnston wrote:
> [...]
> > > Not forgotten. I just hoped there were other reviewers or testers in the
> > > meantime, but be it.
> > > 
> > > Like I said, I don't have FreeBSD system here to test this, so I am taking
> > > your word for now that you tested this and plan to bring this into QEMU
> > > when master re-opens for new features soon.
> > 
> > Thank you very much!
> > 
> > In case I missed somewhat, what testing would you typically do
> > otherwise?  So far I had run the QEMU test suite (which indeed found
> > some bugs in the initial version) and tried mounting a 9pfs share from
> > Linux and FreeBSD guests and doing a bit of manual testing.
> 
> Apart from QEMU's test cases, I also use guest systems running 9p as root file 
> system [1], run software compilations there among some things. That proofed to 
> be quite a useful test environment to spot edge cases, concurrency and 
> performance issues and such.
> 
> [1] https://wiki.qemu.org/Documentation/9p_root_fs

Thanks, I'll give this a try.

> Greg was running some general purpose file system stress test suite in the 
> past, but I currently can't recall what that was.
> 
> > > If you have some time to adjust the commit log message above, that would
> > > be
> > > great, otherwise I can also handle this on my end later on. Looks like
> > > that
> > > comment is not adjusted for v2 yet (i.e. "user." and not mentioning
> > > "system.").
> > 
> > Here's an amended commit log message.  Please let me know if this is
> > better submitted as a v3.
> > 
> > commit b79bf1b7d42025e3e14da86a7c08d269038cd3ed
> > Author: Mark Johnston <markj@FreeBSD.org>
> > Date:   Wed Jul 16 20:32:05 2025 +0000
> > 
> >     9pfs: Add FreeBSD support
> > 
> >     This is largely derived from existing Darwin support.  FreeBSD
> >     apparently has better support for *at() system calls so doesn't require
> >     workarounds for a missing mknodat().  The implementation has a couple of
> > warts however:
> >     - The extattr(2) system calls don't support anything akin to
> >       XATTR_CREATE or XATTR_REPLACE, so a racy workaround is implemented.
> >     - Attribute names cannot begin with "user." or "system." on ZFS, so
> >       these prefixes are trimmed off.  FreeBSD's extattr system calls sport
> >       an extra "namespace" identifier, and attributes created by the 9pfs
> >       backend live in the universal user and system namespaces, so this
> >       seems innocent enough.
> > 
> >     The 9pfs tests were verified to pass on the UFS, ZFS and tmpfs
> >     filesystems.
> > 
> >     Signed-off-by: Mark Johnston <markj@FreeBSD.org>
> 
> Almost. Maybe something like this to make it a bit more clear?
> 
> - Attribute names cannot begin with "user." or "system." on ZFS. However
>   FreeBSD's extattr(2) system supports two dedicated namespaces for these
>   two. So "user." or "system." prefixes are trimmed off from attribute
>   names and instead EXTATTR_NAMESPACE_USER or EXTATTR_NAMESPACE_SYSTEM
>   are picked and passed to extattr system calls instead accordingly.

I folded your suggestion in with a couple of minor tweaks:

commit 61de78986912b03f08354a177caf603857b531b5
Author: Mark Johnston <markj@FreeBSD.org>
Date:   Wed Jul 16 20:32:05 2025 +0000

    9pfs: Add FreeBSD support
    
    This is largely derived from existing Darwin support.  FreeBSD
    apparently has better support for *at() system calls so doesn't require
    workarounds for a missing mknodat().  The implementation has a couple of
    warts however:
    - The extattr(2) system calls don't support anything akin to
      XATTR_CREATE or XATTR_REPLACE, so a racy workaround is implemented.
    - Attribute names cannot begin with "user." or "system." on ZFS.
      However FreeBSD's extattr(2) system calls support two dedicated
      namespaces for these two.  So "user." or "system." prefixes are
      trimmed off from attribute names and instead EXTATTR_NAMESPACE_USER or
      EXTATTR_NAMESPACE_SYSTEM are picked and passed to extattr system calls
      accordingly.
    
    The 9pfs tests were verified to pass on the UFS, ZFS and tmpfs
    filesystems.
    
    Signed-off-by: Mark Johnston <markj@FreeBSD.org>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] 9pfs: Add FreeBSD support
  2025-08-27 14:29       ` Mark Johnston
@ 2025-09-01 10:31         ` Christian Schoenebeck
  0 siblings, 0 replies; 8+ messages in thread
From: Christian Schoenebeck @ 2025-09-01 10:31 UTC (permalink / raw)
  To: qemu-devel, Mark Johnston
  Cc: Greg Kurz, Paolo Bonzini, Marc-André Lureau,
	Daniel P. Berrangé, Philippe Mathieu-Daudé

On Wednesday, August 27, 2025 4:29:37 PM CEST Mark Johnston wrote:
> On Tue, Aug 26, 2025 at 02:34:45PM +0200, Christian Schoenebeck wrote:
[...]
> I folded your suggestion in with a couple of minor tweaks:
> 
> commit 61de78986912b03f08354a177caf603857b531b5
> Author: Mark Johnston <markj@FreeBSD.org>
> Date:   Wed Jul 16 20:32:05 2025 +0000
> 
>     9pfs: Add FreeBSD support
> 
>     This is largely derived from existing Darwin support.  FreeBSD
>     apparently has better support for *at() system calls so doesn't require
>     workarounds for a missing mknodat().  The implementation has a couple of
> warts however:
>     - The extattr(2) system calls don't support anything akin to
>       XATTR_CREATE or XATTR_REPLACE, so a racy workaround is implemented.
>     - Attribute names cannot begin with "user." or "system." on ZFS.
>       However FreeBSD's extattr(2) system calls support two dedicated
>       namespaces for these two.  So "user." or "system." prefixes are
>       trimmed off from attribute names and instead EXTATTR_NAMESPACE_USER or
> EXTATTR_NAMESPACE_SYSTEM are picked and passed to extattr system calls
> accordingly.
> 
>     The 9pfs tests were verified to pass on the UFS, ZFS and tmpfs
>     filesystems.
> 
>     Signed-off-by: Mark Johnston <markj@FreeBSD.org>

LGTM, thanks!

/Christian




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] 9pfs: Add FreeBSD support
  2025-08-26 12:34     ` Christian Schoenebeck
  2025-08-27 14:29       ` Mark Johnston
@ 2025-09-01 10:40       ` Greg Kurz
  1 sibling, 0 replies; 8+ messages in thread
From: Greg Kurz @ 2025-09-01 10:40 UTC (permalink / raw)
  To: Christian Schoenebeck
  Cc: qemu-devel, Mark Johnston, Paolo Bonzini, Marc-André Lureau,
	Daniel P. Berrangé, Philippe Mathieu-Daudé

On Tue, 26 Aug 2025 14:34:45 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Monday, August 25, 2025 2:53:06 PM CEST Mark Johnston wrote:
> > On Thu, Aug 21, 2025 at 01:24:04PM +0200, Christian Schoenebeck wrote:
> > > On Wednesday, August 6, 2025 7:53:08 PM CEST Mark Johnston wrote:
> [...]
> > > Not forgotten. I just hoped there were other reviewers or testers in the
> > > meantime, but be it.
> > > 
> > > Like I said, I don't have FreeBSD system here to test this, so I am taking
> > > your word for now that you tested this and plan to bring this into QEMU
> > > when master re-opens for new features soon.
> > 
> > Thank you very much!
> > 
> > In case I missed somewhat, what testing would you typically do
> > otherwise?  So far I had run the QEMU test suite (which indeed found
> > some bugs in the initial version) and tried mounting a 9pfs share from
> > Linux and FreeBSD guests and doing a bit of manual testing.
> 
> Apart from QEMU's test cases, I also use guest systems running 9p as root file 
> system [1], run software compilations there among some things. That proofed to 
> be quite a useful test environment to spot edge cases, concurrency and 
> performance issues and such.
> 
> [1] https://wiki.qemu.org/Documentation/9p_root_fs
> 
> Greg was running some general purpose file system stress test suite in the 
> past, but I currently can't recall what that was.
> 

Hi Christian and Mark,

I was running this in the guest :

https://github.com/pjd/pjdfstest

Cheers,

--
Greg

> > > If you have some time to adjust the commit log message above, that would
> > > be
> > > great, otherwise I can also handle this on my end later on. Looks like
> > > that
> > > comment is not adjusted for v2 yet (i.e. "user." and not mentioning
> > > "system.").
> > 
> > Here's an amended commit log message.  Please let me know if this is
> > better submitted as a v3.
> > 
> > commit b79bf1b7d42025e3e14da86a7c08d269038cd3ed
> > Author: Mark Johnston <markj@FreeBSD.org>
> > Date:   Wed Jul 16 20:32:05 2025 +0000
> > 
> >     9pfs: Add FreeBSD support
> > 
> >     This is largely derived from existing Darwin support.  FreeBSD
> >     apparently has better support for *at() system calls so doesn't require
> >     workarounds for a missing mknodat().  The implementation has a couple of
> > warts however:
> >     - The extattr(2) system calls don't support anything akin to
> >       XATTR_CREATE or XATTR_REPLACE, so a racy workaround is implemented.
> >     - Attribute names cannot begin with "user." or "system." on ZFS, so
> >       these prefixes are trimmed off.  FreeBSD's extattr system calls sport
> >       an extra "namespace" identifier, and attributes created by the 9pfs
> >       backend live in the universal user and system namespaces, so this
> >       seems innocent enough.
> > 
> >     The 9pfs tests were verified to pass on the UFS, ZFS and tmpfs
> >     filesystems.
> > 
> >     Signed-off-by: Mark Johnston <markj@FreeBSD.org>
> 
> Almost. Maybe something like this to make it a bit more clear?
> 
> - Attribute names cannot begin with "user." or "system." on ZFS. However
>   FreeBSD's extattr(2) system supports two dedicated namespaces for these
>   two. So "user." or "system." prefixes are trimmed off from attribute
>   names and instead EXTATTR_NAMESPACE_USER or EXTATTR_NAMESPACE_SYSTEM
>   are picked and passed to extattr system calls instead accordingly.
> 
> /Christian
> 
> 



-- 
Greg


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] 9pfs: Add FreeBSD support
  2025-08-06 17:53 [PATCH v2] 9pfs: Add FreeBSD support Mark Johnston
  2025-08-21 11:24 ` Christian Schoenebeck
@ 2025-09-05 11:19 ` Christian Schoenebeck
  1 sibling, 0 replies; 8+ messages in thread
From: Christian Schoenebeck @ 2025-09-05 11:19 UTC (permalink / raw)
  To: qemu-devel, Mark Johnston
  Cc: Greg Kurz, Paolo Bonzini, Marc-André Lureau,
	Daniel P. Berrangé, Philippe Mathieu-Daudé

On Wednesday, August 6, 2025 7:53:08 PM CEST Mark Johnston wrote:
> This is largely derived from existing Darwin support.  FreeBSD
> apparently has better support for *at() system calls so doesn't require
> workarounds for a missing mknodat().  The implementation has a couple of
> warts however:
> - The extattr(2) system calls don't support anything akin to
>   XATTR_CREATE or XATTR_REPLACE, so a racy workaround is implemented.
> - Attribute names cannot begin with "user." on ZFS, so the prefix is
>   trimmed off.  FreeBSD's extattr system calls sport an extra
>   "namespace" identifier, and attributes created by the 9pfs backend
>   live in the universal user namespace, so this seems innocent enough.
> 
> The 9pfs tests were verified to pass on the UFS, ZFS and tmpfs
> filesystems.
> 
> Signed-off-by: Mark Johnston <markj@FreeBSD.org>
> ---

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

Thanks!

/Christian




^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-09-05 11:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-06 17:53 [PATCH v2] 9pfs: Add FreeBSD support Mark Johnston
2025-08-21 11:24 ` Christian Schoenebeck
2025-08-25 12:53   ` Mark Johnston
2025-08-26 12:34     ` Christian Schoenebeck
2025-08-27 14:29       ` Mark Johnston
2025-09-01 10:31         ` Christian Schoenebeck
2025-09-01 10:40       ` Greg Kurz
2025-09-05 11:19 ` Christian Schoenebeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).