* [PATCH v9 00/11] 9p: Add support for darwin
@ 2022-02-27 22:35 Will Cohen
2022-02-27 22:35 ` [PATCH v9 01/11] 9p: linux: Fix a couple Linux assumptions Will Cohen
` (11 more replies)
0 siblings, 12 replies; 27+ messages in thread
From: Will Cohen @ 2022-02-27 22:35 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Christian Schoenebeck, Greg Kurz,
Philippe Mathieu-Daudé,
hi, Will Cohen, Paolo Bonzini
This is a followup to https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg04391.html,
adding 9p server support for Darwin.
Since v8, the following changes have been made:
Patch 4/11 (9p: darwin: Handle struct dirent differences)
- Declare qemu_dirent_off as static to prevent linker error
- Move qemu_dirent_off above the end-of-file endif to fix compilation
Patch 9/11 (9p: darwin: Implement compatibility for mknodat)
- Fix line over 90 characters formatting error
- Move qemu_mknodat back from osdep to 9p-util and adjust patch notes accordingly
Patch 11/11 (9p: darwin: meson: Allow VirtFS on Darwin)
- Rebase to master
With these changes, this patch set builds and passes 9p synth tests on both linux and darwin.
Keno Fischer (10):
9p: linux: Fix a couple Linux assumptions
9p: Rename 9p-util -> 9p-util-linux
9p: darwin: Handle struct stat(fs) differences
9p: darwin: Handle struct dirent differences
9p: darwin: Ignore O_{NOATIME, DIRECT}
9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX
9p: darwin: *xattr_nofollow implementations
9p: darwin: Compatibility for f/l*xattr
9p: darwin: Implement compatibility for mknodat
9p: darwin: meson: Allow VirtFS on Darwin
Will Cohen (1):
9p: darwin: Adjust assumption on virtio-9p-test
fsdev/file-op-9p.h | 9 ++-
fsdev/meson.build | 1 +
hw/9pfs/9p-local.c | 27 +++++--
hw/9pfs/9p-proxy.c | 38 +++++++++-
hw/9pfs/9p-synth.c | 6 ++
hw/9pfs/9p-util-darwin.c | 97 ++++++++++++++++++++++++++
hw/9pfs/{9p-util.c => 9p-util-linux.c} | 8 ++-
hw/9pfs/9p-util.h | 46 ++++++++++++
hw/9pfs/9p.c | 42 +++++++++--
hw/9pfs/9p.h | 18 +++++
hw/9pfs/codir.c | 4 +-
hw/9pfs/meson.build | 3 +-
include/qemu/xattr.h | 4 +-
meson.build | 13 ++--
tests/qtest/virtio-9p-test.c | 2 +-
15 files changed, 292 insertions(+), 26 deletions(-)
create mode 100644 hw/9pfs/9p-util-darwin.c
rename hw/9pfs/{9p-util.c => 9p-util-linux.c} (90%)
--
2.35.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v9 01/11] 9p: linux: Fix a couple Linux assumptions
2022-02-27 22:35 [PATCH v9 00/11] 9p: Add support for darwin Will Cohen
@ 2022-02-27 22:35 ` Will Cohen
2022-02-27 22:35 ` [PATCH v9 02/11] 9p: Rename 9p-util -> 9p-util-linux Will Cohen
` (10 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Will Cohen @ 2022-02-27 22:35 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Christian Schoenebeck, Greg Kurz,
Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Will Cohen, Paolo Bonzini, Keno Fischer
From: Keno Fischer <keno@juliacomputing.com>
- Guard Linux only headers.
- Add qemu/statfs.h header to abstract over the which
headers are needed for struct statfs
- Define `ENOATTR` only if not only defined
(it's defined in system headers on Darwin).
Signed-off-by: Keno Fischer <keno@juliacomputing.com>
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
While it might at first appear that fsdev/virtfs-proxy-header.c would
need similar adjustment for darwin as file-op-9p here, a later patch in
this series disables virtfs-proxy-helper for non-Linux. Allowing
virtfs-proxy-helper on darwin could potentially be an additional
optimization later.
[Will Cohen: - Fix headers for Alpine
- Integrate statfs.h back into file-op-9p.h
- Remove superfluous header guards from file-opt-9p
- Add note about virtfs-proxy-helper being disabled
on non-Linux for this patch series]
Signed-off-by: Will Cohen <wwcohen@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
fsdev/file-op-9p.h | 9 ++++++++-
hw/9pfs/9p-local.c | 2 ++
hw/9pfs/9p.c | 4 ++++
include/qemu/xattr.h | 4 +++-
4 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index 8fd89f0447..4997677460 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -16,10 +16,17 @@
#include <dirent.h>
#include <utime.h>
-#include <sys/vfs.h>
#include "qemu-fsdev-throttle.h"
#include "p9array.h"
+#ifdef CONFIG_LINUX
+# include <sys/vfs.h>
+#endif
+#ifdef CONFIG_DARWIN
+# include <sys/param.h>
+# include <sys/mount.h>
+#endif
+
#define SM_LOCAL_MODE_BITS 0600
#define SM_LOCAL_DIR_MODE_BITS 0700
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 210d9e7705..1a5e3eed73 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -32,10 +32,12 @@
#include "qemu/error-report.h"
#include "qemu/option.h"
#include <libgen.h>
+#ifdef CONFIG_LINUX
#include <linux/fs.h>
#ifdef CONFIG_LINUX_MAGIC_H
#include <linux/magic.h>
#endif
+#endif
#include <sys/ioctl.h>
#ifndef XFS_SUPER_MAGIC
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 15b3f4d385..9c63e14b28 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -32,7 +32,11 @@
#include "migration/blocker.h"
#include "qemu/xxhash.h"
#include <math.h>
+#ifdef CONFIG_LINUX
#include <linux/limits.h>
+#else
+#include <limits.h>
+#endif
int open_fd_hw;
int total_open_fd;
diff --git a/include/qemu/xattr.h b/include/qemu/xattr.h
index a83fe8e749..f1d0f7be74 100644
--- a/include/qemu/xattr.h
+++ b/include/qemu/xattr.h
@@ -22,7 +22,9 @@
#ifdef CONFIG_LIBATTR
# include <attr/xattr.h>
#else
-# define ENOATTR ENODATA
+# if !defined(ENOATTR)
+# define ENOATTR ENODATA
+# endif
# include <sys/xattr.h>
#endif
--
2.35.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v9 02/11] 9p: Rename 9p-util -> 9p-util-linux
2022-02-27 22:35 [PATCH v9 00/11] 9p: Add support for darwin Will Cohen
2022-02-27 22:35 ` [PATCH v9 01/11] 9p: linux: Fix a couple Linux assumptions Will Cohen
@ 2022-02-27 22:35 ` Will Cohen
2022-02-27 22:35 ` [PATCH v9 03/11] 9p: darwin: Handle struct stat(fs) differences Will Cohen
` (9 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Will Cohen @ 2022-02-27 22:35 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Christian Schoenebeck, Greg Kurz,
Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Will Cohen, Paolo Bonzini, Keno Fischer
From: Keno Fischer <keno@juliacomputing.com>
The current file only has the Linux versions of these functions.
Rename the file accordingly and update the Makefile to only build
it on Linux. A Darwin version of these will follow later in the
series.
Signed-off-by: Keno Fischer <keno@juliacomputing.com>
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
Signed-off-by: Will Cohen <wwcohen@gmail.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/9pfs/{9p-util.c => 9p-util-linux.c} | 2 +-
hw/9pfs/meson.build | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
rename hw/9pfs/{9p-util.c => 9p-util-linux.c} (97%)
diff --git a/hw/9pfs/9p-util.c b/hw/9pfs/9p-util-linux.c
similarity index 97%
rename from hw/9pfs/9p-util.c
rename to hw/9pfs/9p-util-linux.c
index 3221d9b498..398614a5d0 100644
--- a/hw/9pfs/9p-util.c
+++ b/hw/9pfs/9p-util-linux.c
@@ -1,5 +1,5 @@
/*
- * 9p utilities
+ * 9p utilities (Linux Implementation)
*
* Copyright IBM, Corp. 2017
*
diff --git a/hw/9pfs/meson.build b/hw/9pfs/meson.build
index 99be5d9119..1b28e70040 100644
--- a/hw/9pfs/meson.build
+++ b/hw/9pfs/meson.build
@@ -4,7 +4,6 @@ fs_ss.add(files(
'9p-posix-acl.c',
'9p-proxy.c',
'9p-synth.c',
- '9p-util.c',
'9p-xattr-user.c',
'9p-xattr.c',
'9p.c',
@@ -14,6 +13,7 @@ fs_ss.add(files(
'coth.c',
'coxattr.c',
))
+fs_ss.add(when: 'CONFIG_LINUX', if_true: files('9p-util-linux.c'))
fs_ss.add(when: 'CONFIG_XEN', if_true: files('xen-9p-backend.c'))
softmmu_ss.add_all(when: 'CONFIG_FSDEV_9P', if_true: fs_ss)
--
2.35.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v9 03/11] 9p: darwin: Handle struct stat(fs) differences
2022-02-27 22:35 [PATCH v9 00/11] 9p: Add support for darwin Will Cohen
2022-02-27 22:35 ` [PATCH v9 01/11] 9p: linux: Fix a couple Linux assumptions Will Cohen
2022-02-27 22:35 ` [PATCH v9 02/11] 9p: Rename 9p-util -> 9p-util-linux Will Cohen
@ 2022-02-27 22:35 ` Will Cohen
2022-02-27 22:35 ` [PATCH v9 04/11] 9p: darwin: Handle struct dirent differences Will Cohen
` (8 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Will Cohen @ 2022-02-27 22:35 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Christian Schoenebeck, Greg Kurz,
Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Will Cohen, Paolo Bonzini, Keno Fischer
From: Keno Fischer <keno@juliacomputing.com>
Signed-off-by: Keno Fischer <keno@juliacomputing.com>
Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
[Will Cohen: - Note lack of f_namelen and f_frsize on Darwin
- Ensure that tv_sec and tv_nsec are both
initialized for Darwin and non-Darwin]
Signed-off-by: Will Cohen <wwcohen@gmail.com>
---
hw/9pfs/9p-proxy.c | 22 ++++++++++++++++++++--
hw/9pfs/9p-synth.c | 2 ++
hw/9pfs/9p.c | 16 ++++++++++++++--
3 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
index 09bd9f1464..b1664080d8 100644
--- a/hw/9pfs/9p-proxy.c
+++ b/hw/9pfs/9p-proxy.c
@@ -123,10 +123,16 @@ static void prstatfs_to_statfs(struct statfs *stfs, ProxyStatFS *prstfs)
stfs->f_bavail = prstfs->f_bavail;
stfs->f_files = prstfs->f_files;
stfs->f_ffree = prstfs->f_ffree;
+#ifdef CONFIG_DARWIN
+ /* f_namelen and f_frsize do not exist on Darwin */
+ stfs->f_fsid.val[0] = prstfs->f_fsid[0] & 0xFFFFFFFFU;
+ stfs->f_fsid.val[1] = prstfs->f_fsid[1] >> 32 & 0xFFFFFFFFU;
+#else
stfs->f_fsid.__val[0] = prstfs->f_fsid[0] & 0xFFFFFFFFU;
stfs->f_fsid.__val[1] = prstfs->f_fsid[1] >> 32 & 0xFFFFFFFFU;
stfs->f_namelen = prstfs->f_namelen;
stfs->f_frsize = prstfs->f_frsize;
+#endif
}
/* Converts proxy_stat structure to VFS stat structure */
@@ -143,12 +149,24 @@ static void prstat_to_stat(struct stat *stbuf, ProxyStat *prstat)
stbuf->st_size = prstat->st_size;
stbuf->st_blksize = prstat->st_blksize;
stbuf->st_blocks = prstat->st_blocks;
+ stbuf->st_atime = prstat->st_atim_sec;
+ stbuf->st_mtime = prstat->st_mtim_sec;
+ stbuf->st_ctime = prstat->st_ctim_sec;
+#ifdef CONFIG_DARWIN
+ stbuf->st_atimespec.tv_sec = prstat->st_atim_sec;
+ stbuf->st_mtimespec.tv_sec = prstat->st_mtim_sec;
+ stbuf->st_ctimespec.tv_sec = prstat->st_ctim_sec;
+ stbuf->st_atimespec.tv_nsec = prstat->st_atim_nsec;
+ stbuf->st_mtimespec.tv_nsec = prstat->st_mtim_nsec;
+ stbuf->st_ctimespec.tv_nsec = prstat->st_ctim_nsec;
+#else
stbuf->st_atim.tv_sec = prstat->st_atim_sec;
+ stbuf->st_mtim.tv_sec = prstat->st_mtim_sec;
+ stbuf->st_ctim.tv_sec = prstat->st_ctim_sec;
stbuf->st_atim.tv_nsec = prstat->st_atim_nsec;
- stbuf->st_mtime = prstat->st_mtim_sec;
stbuf->st_mtim.tv_nsec = prstat->st_mtim_nsec;
- stbuf->st_ctime = prstat->st_ctim_sec;
stbuf->st_ctim.tv_nsec = prstat->st_ctim_nsec;
+#endif
}
/*
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index 7a7cd5c5ba..bf9b0c5ddd 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -439,7 +439,9 @@ 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
stbuf->f_namelen = NAME_MAX;
+#endif
return 0;
}
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 9c63e14b28..1563d7b7c6 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1313,11 +1313,17 @@ static int stat_to_v9stat_dotl(V9fsPDU *pdu, const struct stat *stbuf,
v9lstat->st_blksize = stat_to_iounit(pdu, stbuf);
v9lstat->st_blocks = stbuf->st_blocks;
v9lstat->st_atime_sec = stbuf->st_atime;
- v9lstat->st_atime_nsec = stbuf->st_atim.tv_nsec;
v9lstat->st_mtime_sec = stbuf->st_mtime;
- v9lstat->st_mtime_nsec = stbuf->st_mtim.tv_nsec;
v9lstat->st_ctime_sec = stbuf->st_ctime;
+#ifdef CONFIG_DARWIN
+ v9lstat->st_atime_nsec = stbuf->st_atimespec.tv_nsec;
+ v9lstat->st_mtime_nsec = stbuf->st_mtimespec.tv_nsec;
+ v9lstat->st_ctime_nsec = stbuf->st_ctimespec.tv_nsec;
+#else
+ v9lstat->st_atime_nsec = stbuf->st_atim.tv_nsec;
+ v9lstat->st_mtime_nsec = stbuf->st_mtim.tv_nsec;
v9lstat->st_ctime_nsec = stbuf->st_ctim.tv_nsec;
+#endif
/* Currently we only support BASIC fields in stat */
v9lstat->st_result_mask = P9_STATS_BASIC;
@@ -3519,9 +3525,15 @@ 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
+ fsid_val = (unsigned int)stbuf->f_fsid.val[0] |
+ (unsigned long long)stbuf->f_fsid.val[1] << 32;
+ f_namelen = NAME_MAX;
+#else
fsid_val = (unsigned int) stbuf->f_fsid.__val[0] |
(unsigned long long)stbuf->f_fsid.__val[1] << 32;
f_namelen = stbuf->f_namelen;
+#endif
return pdu_marshal(pdu, offset, "ddqqqqqqd",
f_type, f_bsize, f_blocks, f_bfree,
--
2.35.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v9 04/11] 9p: darwin: Handle struct dirent differences
2022-02-27 22:35 [PATCH v9 00/11] 9p: Add support for darwin Will Cohen
` (2 preceding siblings ...)
2022-02-27 22:35 ` [PATCH v9 03/11] 9p: darwin: Handle struct stat(fs) differences Will Cohen
@ 2022-02-27 22:35 ` Will Cohen
2022-02-27 22:35 ` [PATCH v9 05/11] 9p: darwin: Ignore O_{NOATIME, DIRECT} Will Cohen
` (7 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Will Cohen @ 2022-02-27 22:35 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Fabian Franz, Christian Schoenebeck,
Greg Kurz, Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Will Cohen, Paolo Bonzini, Keno Fischer
From: Keno Fischer <keno@juliacomputing.com>
On darwin d_seekoff exists, but is optional and does not seem to
be commonly used by file systems. Use `telldir` instead to obtain
the seek offset and inject it into d_seekoff, and create a
qemu_dirent_off helper to call it appropriately when appropriate.
Signed-off-by: Keno Fischer <keno@juliacomputing.com>
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
[Will Cohen: - Adjust to pass testing
- Ensure that d_seekoff is filled using telldir
on darwin, and create qemu_dirent_off helper
to decide which to access]
[Fabian Franz: - Add telldir error handling for darwin]
Signed-off-by: Fabian Franz <fabianfranz.oss@gmail.com>
[Will Cohen: - Ensure that telldir error handling uses
signed int
- Cleanup of telldir error handling
- Remove superfluous error handling for
qemu_dirent_off
- Adjust formatting
- Use qemu_dirent_off in codir.c
- Declare qemu_dirent_off as static to prevent
linker error
- Move qemu_dirent_off above the end-of-file
endif to fix compilation]
Signed-off-by: Will Cohen <wwcohen@gmail.com>
---
hw/9pfs/9p-local.c | 9 +++++++++
hw/9pfs/9p-proxy.c | 16 +++++++++++++++-
hw/9pfs/9p-synth.c | 4 ++++
hw/9pfs/9p-util.h | 16 ++++++++++++++++
hw/9pfs/9p.c | 7 +++++--
hw/9pfs/codir.c | 4 +++-
6 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 1a5e3eed73..f3272f0b43 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -562,6 +562,15 @@ again:
if (!entry) {
return NULL;
}
+#ifdef CONFIG_DARWIN
+ int off;
+ off = telldir(fs->dir.stream);
+ /* If telldir fails, fail the entire readdir call */
+ if (off < 0) {
+ return NULL;
+ }
+ entry->d_seekoff = off;
+#endif
if (ctx->export_flags & V9FS_SM_MAPPED) {
entry->d_type = DT_UNKNOWN;
diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
index b1664080d8..8b4b5cf7dc 100644
--- a/hw/9pfs/9p-proxy.c
+++ b/hw/9pfs/9p-proxy.c
@@ -706,7 +706,21 @@ static off_t proxy_telldir(FsContext *ctx, V9fsFidOpenState *fs)
static struct dirent *proxy_readdir(FsContext *ctx, V9fsFidOpenState *fs)
{
- return readdir(fs->dir.stream);
+ struct dirent *entry;
+ entry = readdir(fs->dir.stream);
+#ifdef CONFIG_DARWIN
+ if (!entry) {
+ return NULL;
+ }
+ int td;
+ td = telldir(fs->dir.stream);
+ /* If telldir fails, fail the entire readdir call */
+ if (td < 0) {
+ return NULL;
+ }
+ entry->d_seekoff = td;
+#endif
+ return entry;
}
static void proxy_seekdir(FsContext *ctx, V9fsFidOpenState *fs, off_t off)
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index bf9b0c5ddd..b3080e415b 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -234,7 +234,11 @@ static void synth_direntry(V9fsSynthNode *node,
offsetof(struct dirent, d_name) + sz);
memcpy(entry->d_name, node->name, sz);
entry->d_ino = node->attr->inode;
+#ifdef CONFIG_DARWIN
+ entry->d_seekoff = off + 1;
+#else
entry->d_off = off + 1;
+#endif
}
static struct dirent *synth_get_dentry(V9fsSynthNode *dir,
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 546f46dc7d..7449733c15 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -78,4 +78,20 @@ ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
const char *name);
+/**
+ * Darwin has d_seekoff, which appears to function similarly to d_off.
+ * However, it does not appear to be supported on all file systems,
+ * so ensure it is manually injected earlier and call here when
+ * needed.
+ */
+static inline off_t qemu_dirent_off(struct dirent *dent)
+{
+#ifdef CONFIG_DARWIN
+ return dent->d_seekoff;
+#else
+ return dent->d_off;
+#endif
+}
+
+
#endif
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 1563d7b7c6..caf3b240fe 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -27,6 +27,7 @@
#include "virtio-9p.h"
#include "fsdev/qemu-fsdev.h"
#include "9p-xattr.h"
+#include "9p-util.h"
#include "coth.h"
#include "trace.h"
#include "migration/blocker.h"
@@ -2281,7 +2282,7 @@ static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu,
count += len;
v9fs_stat_free(&v9stat);
v9fs_path_free(&path);
- saved_dir_pos = dent->d_off;
+ saved_dir_pos = qemu_dirent_off(dent);
}
v9fs_readdir_unlock(&fidp->fs.dir);
@@ -2420,6 +2421,7 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
V9fsString name;
int len, err = 0;
int32_t count = 0;
+ off_t off;
struct dirent *dent;
struct stat *st;
struct V9fsDirEnt *entries = NULL;
@@ -2480,12 +2482,13 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
qid.version = 0;
}
+ off = qemu_dirent_off(dent);
v9fs_string_init(&name);
v9fs_string_sprintf(&name, "%s", dent->d_name);
/* 11 = 7 + 4 (7 = start offset, 4 = space for storing count) */
len = pdu_marshal(pdu, 11 + count, "Qqbs",
- &qid, dent->d_off,
+ &qid, off,
dent->d_type, &name);
v9fs_string_free(&name);
diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
index c0873bde16..f96d8ac4e6 100644
--- a/hw/9pfs/codir.c
+++ b/hw/9pfs/codir.c
@@ -22,6 +22,8 @@
#include "qemu/coroutine.h"
#include "qemu/main-loop.h"
#include "coth.h"
+#include "9p-xattr.h"
+#include "9p-util.h"
/*
* Intended to be called from bottom-half (e.g. background I/O thread)
@@ -166,7 +168,7 @@ static int do_readdir_many(V9fsPDU *pdu, V9fsFidState *fidp,
}
size += len;
- saved_dir_pos = dent->d_off;
+ saved_dir_pos = qemu_dirent_off(dent);
}
/* restore (last) saved position */
--
2.35.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v9 05/11] 9p: darwin: Ignore O_{NOATIME, DIRECT}
2022-02-27 22:35 [PATCH v9 00/11] 9p: Add support for darwin Will Cohen
` (3 preceding siblings ...)
2022-02-27 22:35 ` [PATCH v9 04/11] 9p: darwin: Handle struct dirent differences Will Cohen
@ 2022-02-27 22:35 ` Will Cohen
2022-02-27 22:35 ` [PATCH v9 06/11] 9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX Will Cohen
` (6 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Will Cohen @ 2022-02-27 22:35 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Christian Schoenebeck, Greg Kurz,
Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Will Cohen, Paolo Bonzini, Keno Fischer
From: Keno Fischer <keno@juliacomputing.com>
Darwin doesn't have either of these flags. Darwin does have
F_NOCACHE, which is similar to O_DIRECT, but has different
enough semantics that other projects don't generally map
them automatically. In any case, we don't support O_DIRECT
on Linux at the moment either.
Signed-off-by: Keno Fischer <keno@juliacomputing.com>
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
[Will Cohen: - Adjust coding style]
Signed-off-by: Will Cohen <wwcohen@gmail.com>
---
hw/9pfs/9p-util.h | 2 ++
hw/9pfs/9p.c | 13 ++++++++++++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 7449733c15..2b9ac74291 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -41,6 +41,7 @@ again:
fd = openat(dirfd, name, flags | O_NOFOLLOW | O_NOCTTY | O_NONBLOCK,
mode);
if (fd == -1) {
+#ifndef CONFIG_DARWIN
if (errno == EPERM && (flags & O_NOATIME)) {
/*
* The client passed O_NOATIME but we lack permissions to honor it.
@@ -53,6 +54,7 @@ again:
flags &= ~O_NOATIME;
goto again;
}
+#endif
return -1;
}
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index caf3b240fe..14e84c3bcf 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -138,11 +138,20 @@ 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
+ { P9_DOTL_NOATIME, O_NOATIME },
+ /*
+ * On Darwin, we could map to F_NOCACHE, which is
+ * similar, but doesn't quite have the same
+ * semantics. However, we don't support O_DIRECT
+ * even on linux at the moment, so we just ignore
+ * it here.
+ */
{ P9_DOTL_DIRECT, O_DIRECT },
+#endif
{ P9_DOTL_LARGEFILE, O_LARGEFILE },
{ P9_DOTL_DIRECTORY, O_DIRECTORY },
{ P9_DOTL_NOFOLLOW, O_NOFOLLOW },
- { P9_DOTL_NOATIME, O_NOATIME },
{ P9_DOTL_SYNC, O_SYNC },
};
@@ -171,10 +180,12 @@ static int get_dotl_openflags(V9fsState *s, int oflags)
*/
flags = dotl_to_open_flags(oflags);
flags &= ~(O_NOCTTY | O_ASYNC | O_CREAT);
+#ifndef CONFIG_DARWIN
/*
* Ignore direct disk access hint until the server supports it.
*/
flags &= ~O_DIRECT;
+#endif
return flags;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v9 06/11] 9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX
2022-02-27 22:35 [PATCH v9 00/11] 9p: Add support for darwin Will Cohen
` (4 preceding siblings ...)
2022-02-27 22:35 ` [PATCH v9 05/11] 9p: darwin: Ignore O_{NOATIME, DIRECT} Will Cohen
@ 2022-02-27 22:35 ` Will Cohen
2022-02-27 22:35 ` [PATCH v9 07/11] 9p: darwin: *xattr_nofollow implementations Will Cohen
` (5 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Will Cohen @ 2022-02-27 22:35 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Fabian Franz, Christian Schoenebeck,
Greg Kurz, Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Will Cohen, Paolo Bonzini, Keno Fischer
From: Keno Fischer <keno@juliacomputing.com>
Signed-off-by: Keno Fischer <keno@juliacomputing.com>
Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
Because XATTR_SIZE_MAX is not defined on Darwin,
create a cross-platform P9_XATTR_SIZE_MAX instead.
[Will Cohen: - Adjust coding style
- Lower XATTR_SIZE_MAX to 64k
- Add explanatory context related to XATTR_SIZE_MAX]
[Fabian Franz: - Move XATTR_SIZE_MAX reference from 9p.c to
P9_XATTR_SIZE_MAX in 9p.h]
Signed-off-by: Will Cohen <wwcohen@gmail.com>
Signed-off-by: Fabian Franz <fabianfranz.oss@gmail.com>
[Will Cohen: - For P9_XATTR_MAX, ensure that Linux uses
XATTR_SIZE_MAX, Darwin uses 64k, and error
out for undefined hosts]
Signed-off-by: Will Cohen <wwcohen@gmail.com>
---
hw/9pfs/9p.c | 2 +-
hw/9pfs/9p.h | 18 ++++++++++++++++++
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 14e84c3bcf..7405352c37 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3949,7 +3949,7 @@ static void coroutine_fn v9fs_xattrcreate(void *opaque)
rflags |= XATTR_REPLACE;
}
- if (size > XATTR_SIZE_MAX) {
+ if (size > P9_XATTR_SIZE_MAX) {
err = -E2BIG;
goto out_nofid;
}
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 1567b67841..94b273b3d0 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -479,4 +479,22 @@ struct V9fsTransport {
void (*push_and_notify)(V9fsPDU *pdu);
};
+#if defined(XATTR_SIZE_MAX)
+/* Linux */
+#define P9_XATTR_SIZE_MAX XATTR_SIZE_MAX
+#elif defined(CONFIG_DARWIN)
+/*
+ * Darwin doesn't seem to define a maximum xattr size in its user
+ * space header, so manually configure it across platforms as 64k.
+ *
+ * Having no limit at all can lead to QEMU crashing during large g_malloc()
+ * calls. Because QEMU does not currently support macOS guests, the below
+ * preliminary solution only works due to its being a reflection of the limit of
+ * Linux guests.
+ */
+#define P9_XATTR_SIZE_MAX 65536
+#else
+#error Missing definition for P9_XATTR_SIZE_MAX for this host system
+#endif
+
#endif
--
2.35.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v9 07/11] 9p: darwin: *xattr_nofollow implementations
2022-02-27 22:35 [PATCH v9 00/11] 9p: Add support for darwin Will Cohen
` (5 preceding siblings ...)
2022-02-27 22:35 ` [PATCH v9 06/11] 9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX Will Cohen
@ 2022-02-27 22:35 ` Will Cohen
2022-02-27 22:35 ` [PATCH v9 08/11] 9p: darwin: Compatibility for f/l*xattr Will Cohen
` (4 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Will Cohen @ 2022-02-27 22:35 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Christian Schoenebeck, Greg Kurz,
Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Will Cohen, Paolo Bonzini, Keno Fischer
From: Keno Fischer <keno@juliacomputing.com>
This implements the darwin equivalent of the functions that were
moved to 9p-util(-linux) earlier in this series in the new
9p-util-darwin file.
Signed-off-by: Keno Fischer <keno@juliacomputing.com>
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
Signed-off-by: Will Cohen <wwcohen@gmail.com>
---
hw/9pfs/9p-util-darwin.c | 64 ++++++++++++++++++++++++++++++++++++++++
hw/9pfs/meson.build | 1 +
2 files changed, 65 insertions(+)
create mode 100644 hw/9pfs/9p-util-darwin.c
diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
new file mode 100644
index 0000000000..cdb4c9e24c
--- /dev/null
+++ b/hw/9pfs/9p-util-darwin.c
@@ -0,0 +1,64 @@
+/*
+ * 9p utilities (Darwin 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.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/xattr.h"
+#include "9p-util.h"
+
+ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char *name,
+ void *value, size_t size)
+{
+ int ret;
+ int fd = openat_file(dirfd, filename,
+ O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+ if (fd == -1) {
+ return -1;
+ }
+ ret = fgetxattr(fd, name, value, size, 0, 0);
+ close_preserve_errno(fd);
+ return ret;
+}
+
+ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
+ char *list, size_t size)
+{
+ int ret;
+ int fd = openat_file(dirfd, filename,
+ O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+ if (fd == -1) {
+ return -1;
+ }
+ ret = flistxattr(fd, list, size, 0);
+ close_preserve_errno(fd);
+ return ret;
+}
+
+ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
+ const char *name)
+{
+ int ret;
+ int fd = openat_file(dirfd, filename, O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+ if (fd == -1) {
+ return -1;
+ }
+ ret = fremovexattr(fd, name, 0);
+ close_preserve_errno(fd);
+ return ret;
+}
+
+int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
+ void *value, size_t size, int flags)
+{
+ int ret;
+ int fd = openat_file(dirfd, filename, O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+ if (fd == -1) {
+ return -1;
+ }
+ ret = fsetxattr(fd, name, value, size, 0, flags);
+ close_preserve_errno(fd);
+ return ret;
+}
diff --git a/hw/9pfs/meson.build b/hw/9pfs/meson.build
index 1b28e70040..12443b6ad5 100644
--- a/hw/9pfs/meson.build
+++ b/hw/9pfs/meson.build
@@ -14,6 +14,7 @@ fs_ss.add(files(
'coxattr.c',
))
fs_ss.add(when: 'CONFIG_LINUX', if_true: files('9p-util-linux.c'))
+fs_ss.add(when: 'CONFIG_DARWIN', if_true: files('9p-util-darwin.c'))
fs_ss.add(when: 'CONFIG_XEN', if_true: files('xen-9p-backend.c'))
softmmu_ss.add_all(when: 'CONFIG_FSDEV_9P', if_true: fs_ss)
--
2.35.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v9 08/11] 9p: darwin: Compatibility for f/l*xattr
2022-02-27 22:35 [PATCH v9 00/11] 9p: Add support for darwin Will Cohen
` (6 preceding siblings ...)
2022-02-27 22:35 ` [PATCH v9 07/11] 9p: darwin: *xattr_nofollow implementations Will Cohen
@ 2022-02-27 22:35 ` Will Cohen
2022-02-27 22:35 ` [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat Will Cohen
` (3 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Will Cohen @ 2022-02-27 22:35 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Christian Schoenebeck, Greg Kurz,
Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Will Cohen, Paolo Bonzini, Keno Fischer
From: Keno Fischer <keno@juliacomputing.com>
On darwin `fgetxattr` takes two extra optional arguments,
and the l* variants are not defined (in favor of an extra
flag to the regular variants.
Signed-off-by: Keno Fischer <keno@juliacomputing.com>
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
Signed-off-by: Will Cohen <wwcohen@gmail.com>
---
hw/9pfs/9p-local.c | 12 ++++++++----
hw/9pfs/9p-util.h | 17 +++++++++++++++++
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index f3272f0b43..a0d08e5216 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -790,16 +790,20 @@ static int local_fstat(FsContext *fs_ctx, int fid_type,
mode_t tmp_mode;
dev_t tmp_dev;
- if (fgetxattr(fd, "user.virtfs.uid", &tmp_uid, sizeof(uid_t)) > 0) {
+ if (qemu_fgetxattr(fd, "user.virtfs.uid",
+ &tmp_uid, sizeof(uid_t)) > 0) {
stbuf->st_uid = le32_to_cpu(tmp_uid);
}
- if (fgetxattr(fd, "user.virtfs.gid", &tmp_gid, sizeof(gid_t)) > 0) {
+ if (qemu_fgetxattr(fd, "user.virtfs.gid",
+ &tmp_gid, sizeof(gid_t)) > 0) {
stbuf->st_gid = le32_to_cpu(tmp_gid);
}
- if (fgetxattr(fd, "user.virtfs.mode", &tmp_mode, sizeof(mode_t)) > 0) {
+ if (qemu_fgetxattr(fd, "user.virtfs.mode",
+ &tmp_mode, sizeof(mode_t)) > 0) {
stbuf->st_mode = le32_to_cpu(tmp_mode);
}
- if (fgetxattr(fd, "user.virtfs.rdev", &tmp_dev, sizeof(dev_t)) > 0) {
+ if (qemu_fgetxattr(fd, "user.virtfs.rdev",
+ &tmp_dev, sizeof(dev_t)) > 0) {
stbuf->st_rdev = le64_to_cpu(tmp_dev);
}
} else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 2b9ac74291..9abff79884 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -19,6 +19,23 @@
#define O_PATH_9P_UTIL 0
#endif
+#ifdef CONFIG_DARWIN
+#define qemu_fgetxattr(...) fgetxattr(__VA_ARGS__, 0, 0)
+#define qemu_lgetxattr(...) getxattr(__VA_ARGS__, 0, XATTR_NOFOLLOW)
+#define qemu_llistxattr(...) listxattr(__VA_ARGS__, XATTR_NOFOLLOW)
+#define qemu_lremovexattr(...) removexattr(__VA_ARGS__, XATTR_NOFOLLOW)
+static inline int qemu_lsetxattr(const char *path, const char *name,
+ const void *value, size_t size, int flags) {
+ return setxattr(path, name, value, size, 0, flags | XATTR_NOFOLLOW);
+}
+#else
+#define qemu_fgetxattr fgetxattr
+#define qemu_lgetxattr lgetxattr
+#define qemu_llistxattr llistxattr
+#define qemu_lremovexattr lremovexattr
+#define qemu_lsetxattr lsetxattr
+#endif
+
static inline void close_preserve_errno(int fd)
{
int serrno = errno;
--
2.35.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat
2022-02-27 22:35 [PATCH v9 00/11] 9p: Add support for darwin Will Cohen
` (7 preceding siblings ...)
2022-02-27 22:35 ` [PATCH v9 08/11] 9p: darwin: Compatibility for f/l*xattr Will Cohen
@ 2022-02-27 22:35 ` Will Cohen
2022-02-28 13:20 ` Christian Schoenebeck
2022-04-08 13:52 ` Christian Schoenebeck
2022-02-27 22:35 ` [PATCH v9 10/11] 9p: darwin: Adjust assumption on virtio-9p-test Will Cohen
` (2 subsequent siblings)
11 siblings, 2 replies; 27+ messages in thread
From: Will Cohen @ 2022-02-27 22:35 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Christian Schoenebeck, Greg Kurz,
Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Will Cohen, Paolo Bonzini, Keno Fischer
From: Keno Fischer <keno@juliacomputing.com>
Darwin does not support mknodat. However, to avoid race conditions
with later setting the permissions, we must avoid using mknod on
the full path instead. We could try to fchdir, but that would cause
problems if multiple threads try to call mknodat at the same time.
However, luckily there is a solution: Darwin includes a function
that sets the cwd for the current thread only.
This should suffice to use mknod safely.
This function (pthread_fchdir_np) is protected by a check in
meson in a patch later in this series.
Signed-off-by: Keno Fischer <keno@juliacomputing.com>
Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
[Will Cohen: - Adjust coding style
- Replace clang references with gcc
- Note radar filed with Apple for missing syscall
- Replace direct syscall with pthread_fchdir_np and
adjust patch notes accordingly
- Declare pthread_fchdir_np with
- __attribute__((weak_import)) to allow checking for
its presence before usage
- Move declarations above cplusplus guard
- Add CONFIG_PTHREAD_FCHDIR_NP to meson and check for
presence in 9p-util
- Rebase to apply cleanly on top of the 2022-02-10
changes to 9pfs
- Fix line over 90 characters formatting error]
Signed-off-by: Will Cohen <wwcohen@gmail.com>
---
hw/9pfs/9p-local.c | 4 ++--
hw/9pfs/9p-util-darwin.c | 33 +++++++++++++++++++++++++++++++++
hw/9pfs/9p-util-linux.c | 6 ++++++
hw/9pfs/9p-util.h | 11 +++++++++++
meson.build | 1 +
5 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index a0d08e5216..d42ce6d8b8 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -682,7 +682,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
if (fs_ctx->export_flags & V9FS_SM_MAPPED ||
fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
- err = mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
+ err = qemu_mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
if (err == -1) {
goto out;
}
@@ -697,7 +697,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
}
} else if (fs_ctx->export_flags & V9FS_SM_PASSTHROUGH ||
fs_ctx->export_flags & V9FS_SM_NONE) {
- err = mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
+ err = qemu_mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
if (err == -1) {
goto out;
}
diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
index cdb4c9e24c..bec0253474 100644
--- a/hw/9pfs/9p-util-darwin.c
+++ b/hw/9pfs/9p-util-darwin.c
@@ -7,6 +7,8 @@
#include "qemu/osdep.h"
#include "qemu/xattr.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
#include "9p-util.h"
ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char *name,
@@ -62,3 +64,34 @@ int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
close_preserve_errno(fd);
return ret;
}
+
+/*
+ * As long as mknodat is not available on macOS, this workaround
+ * using pthread_fchdir_np is needed.
+ *
+ * Radar filed with Apple for implementing mknodat:
+ * rdar://FB9862426 (https://openradar.appspot.com/FB9862426)
+ */
+#if defined CONFIG_PTHREAD_FCHDIR_NP
+
+int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
+{
+ int preserved_errno, err;
+ if (!pthread_fchdir_np) {
+ error_report_once("pthread_fchdir_np() not available on this version of macOS");
+ return -ENOTSUP;
+ }
+ if (pthread_fchdir_np(dirfd) < 0) {
+ return -1;
+ }
+ err = mknod(filename, mode, dev);
+ preserved_errno = errno;
+ /* Stop using the thread-local cwd */
+ pthread_fchdir_np(-1);
+ if (err < 0) {
+ errno = preserved_errno;
+ }
+ return err;
+}
+
+#endif
diff --git a/hw/9pfs/9p-util-linux.c b/hw/9pfs/9p-util-linux.c
index 398614a5d0..db451b0784 100644
--- a/hw/9pfs/9p-util-linux.c
+++ b/hw/9pfs/9p-util-linux.c
@@ -61,4 +61,10 @@ int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
ret = lsetxattr(proc_path, name, value, size, flags);
g_free(proc_path);
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 9abff79884..1f74d37558 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -112,5 +112,16 @@ static inline off_t qemu_dirent_off(struct dirent *dent)
#endif
}
+/*
+ * As long as mknodat is not available on macOS, this workaround
+ * using pthread_fchdir_np is needed. qemu_mknodat is defined in
+ * os-posix.c. pthread_fchdir_np is weakly linked here as a guard
+ * in case it disappears in future macOS versions, because it is
+ * is a private API.
+ */
+#if defined CONFIG_DARWIN && defined CONFIG_PTHREAD_FCHDIR_NP
+int pthread_fchdir_np(int fd) __attribute__((weak_import));
+#endif
+int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev);
#endif
diff --git a/meson.build b/meson.build
index 8df40bfac4..3f8dca2c7a 100644
--- a/meson.build
+++ b/meson.build
@@ -1609,6 +1609,7 @@ config_host_data.set('CONFIG_POSIX_FALLOCATE', cc.has_function('posix_fallocate'
config_host_data.set('CONFIG_POSIX_MEMALIGN', cc.has_function('posix_memalign'))
config_host_data.set('CONFIG_PPOLL', cc.has_function('ppoll'))
config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix: '#include <sys/uio.h>'))
+config_host_data.set('CONFIG_PTHREAD_FCHDIR_NP', cc.has_function('pthread_fchdir_np'))
config_host_data.set('CONFIG_SEM_TIMEDWAIT', cc.has_function('sem_timedwait', dependencies: threads))
config_host_data.set('CONFIG_SENDFILE', cc.has_function('sendfile'))
config_host_data.set('CONFIG_SETNS', cc.has_function('setns') and cc.has_function('unshare'))
--
2.35.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v9 10/11] 9p: darwin: Adjust assumption on virtio-9p-test
2022-02-27 22:35 [PATCH v9 00/11] 9p: Add support for darwin Will Cohen
` (8 preceding siblings ...)
2022-02-27 22:35 ` [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat Will Cohen
@ 2022-02-27 22:35 ` Will Cohen
2022-02-27 22:35 ` [PATCH v9 11/11] 9p: darwin: meson: Allow VirtFS on Darwin Will Cohen
2022-03-01 19:25 ` [PATCH v9 00/11] 9p: Add support for darwin Christian Schoenebeck
11 siblings, 0 replies; 27+ messages in thread
From: Will Cohen @ 2022-02-27 22:35 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Fabian Franz, Christian Schoenebeck,
Greg Kurz, Philippe Mathieu-Daudé,
hi, Will Cohen, Paolo Bonzini
The previous test depended on the assumption that P9_DOTL_AT_REMOVEDIR
and AT_REMOVEDIR have the same value.
While this is true on Linux, it is not true everywhere, and leads to an
incorrect test failure on unlink_at, noticed when adding 9p to darwin:
Received response 7 (RLERROR) instead of 77 (RUNLINKAT)
Rlerror has errno 22 (Invalid argument)
**
ERROR:../tests/qtest/virtio-9p-test.c:305:v9fs_req_recv: assertion
failed (hdr.id == id): (7 == 77) Bail out!
ERROR:../tests/qtest/virtio-9p-test.c:305:v9fs_req_recv: assertion
failed (hdr.id == id): (7 == 77)
Signed-off-by: Fabian Franz <fabianfranz.oss@gmail.com>
[Will Cohen: - Add explanation of patch and description
of pre-patch test failure]
Signed-off-by: Will Cohen <wwcohen@gmail.com>
Acked-by: Thomas Huth <thuth@redhat.com>
[Will Cohen: - Move this patch before 9p: darwin: meson
patch to avoid qtest breakage during
bisecting]
Signed-off-by: Will Cohen <wwcohen@gmail.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
tests/qtest/virtio-9p-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 502e5ad0c7..01ca076afe 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -1253,7 +1253,7 @@ static void fs_unlinkat_dir(void *obj, void *data, QGuestAllocator *t_alloc)
/* ... and is actually a directory */
g_assert((st.st_mode & S_IFMT) == S_IFDIR);
- do_unlinkat(v9p, "/", "02", AT_REMOVEDIR);
+ do_unlinkat(v9p, "/", "02", P9_DOTL_AT_REMOVEDIR);
/* directory should be gone now */
g_assert(stat(new_dir, &st) != 0);
}
--
2.35.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v9 11/11] 9p: darwin: meson: Allow VirtFS on Darwin
2022-02-27 22:35 [PATCH v9 00/11] 9p: Add support for darwin Will Cohen
` (9 preceding siblings ...)
2022-02-27 22:35 ` [PATCH v9 10/11] 9p: darwin: Adjust assumption on virtio-9p-test Will Cohen
@ 2022-02-27 22:35 ` Will Cohen
2022-02-28 13:11 ` Christian Schoenebeck
2022-03-01 19:25 ` [PATCH v9 00/11] 9p: Add support for darwin Christian Schoenebeck
11 siblings, 1 reply; 27+ messages in thread
From: Will Cohen @ 2022-02-27 22:35 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Christian Schoenebeck, Greg Kurz,
Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Will Cohen, Paolo Bonzini, Keno Fischer
From: Keno Fischer <keno@juliacomputing.com>
To allow VirtFS on darwin, we need to check that pthread_fchdir_np is
available, which has only been available since macOS 10.12.
Additionally, virtfs_proxy_helper is disabled on Darwin. This patch
series does not currently provide an implementation of the proxy-helper,
but this functionality could be implemented later on.
Signed-off-by: Keno Fischer <keno@juliacomputing.com>
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
[Will Cohen: - Rebase to master]
Signed-off-by: Will Cohen <wwcohen@gmail.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
[Will Cohen: - Add check for pthread_fchdir_np to virtfs
- Add comments to patch commit
- Note that virtfs_proxy_helper does not work
on macOS
- Fully adjust meson virtfs error note to specify
macOS
- Rebase to master]
Signed-off-by: Will Cohen <wwcohen@gmail.com>
---
fsdev/meson.build | 1 +
meson.build | 12 +++++++-----
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/fsdev/meson.build b/fsdev/meson.build
index adf57cc43e..b632b66348 100644
--- a/fsdev/meson.build
+++ b/fsdev/meson.build
@@ -7,6 +7,7 @@ fsdev_ss.add(when: ['CONFIG_FSDEV_9P'], if_true: files(
'qemu-fsdev.c',
), if_false: files('qemu-fsdev-dummy.c'))
softmmu_ss.add_all(when: 'CONFIG_LINUX', if_true: fsdev_ss)
+softmmu_ss.add_all(when: 'CONFIG_DARWIN', if_true: fsdev_ss)
if have_virtfs_proxy_helper
executable('virtfs-proxy-helper',
diff --git a/meson.build b/meson.build
index 3f8dca2c7a..ba52ed9e9a 100644
--- a/meson.build
+++ b/meson.build
@@ -1450,14 +1450,16 @@ dbus_display = get_option('dbus_display') \
.allowed()
have_virtfs = get_option('virtfs') \
- .require(targetos == 'linux',
- error_message: 'virtio-9p (virtfs) requires Linux') \
- .require(libattr.found() and libcap_ng.found(),
- error_message: 'virtio-9p (virtfs) requires libcap-ng-devel and libattr-devel') \
+ .require(targetos == 'linux' or targetos == 'darwin',
+ error_message: 'virtio-9p (virtfs) requires Linux or macOS') \
+ .require(targetos == 'linux' or cc.has_function('pthread_fchdir_np'),
+ error_message: 'virtio-9p (virtfs) on macOS requires the presence of pthread_fchdir_np') \
+ .require(targetos == 'darwin' or (libattr.found() and libcap_ng.found()),
+ error_message: 'virtio-9p (virtfs) on Linux requires libcap-ng-devel and libattr-devel') \
.disable_auto_if(not have_tools and not have_system) \
.allowed()
-have_virtfs_proxy_helper = have_virtfs and have_tools
+have_virtfs_proxy_helper = targetos != 'darwin' and have_virtfs and have_tools
foreach k : get_option('trace_backends')
config_host_data.set('CONFIG_TRACE_' + k.to_upper(), true)
--
2.35.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v9 11/11] 9p: darwin: meson: Allow VirtFS on Darwin
2022-02-27 22:35 ` [PATCH v9 11/11] 9p: darwin: meson: Allow VirtFS on Darwin Will Cohen
@ 2022-02-28 13:11 ` Christian Schoenebeck
2022-02-28 13:43 ` Will Cohen
0 siblings, 1 reply; 27+ messages in thread
From: Christian Schoenebeck @ 2022-02-28 13:11 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini
Cc: Will Cohen, Laurent Vivier, Thomas Huth, Greg Kurz,
Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Keno Fischer
On Sonntag, 27. Februar 2022 23:35:22 CET Will Cohen wrote:
> From: Keno Fischer <keno@juliacomputing.com>
>
> To allow VirtFS on darwin, we need to check that pthread_fchdir_np is
> available, which has only been available since macOS 10.12.
>
> Additionally, virtfs_proxy_helper is disabled on Darwin. This patch
> series does not currently provide an implementation of the proxy-helper,
> but this functionality could be implemented later on.
>
> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> [Michael Roitzsch: - Rebase for NixOS]
> Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
> [Will Cohen: - Rebase to master]
> Signed-off-by: Will Cohen <wwcohen@gmail.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo, could you have a look at this patch, please? It has changed quite a bit
since your review.
Best regards,
Christian Schoenebeck
> [Will Cohen: - Add check for pthread_fchdir_np to virtfs
> - Add comments to patch commit
> - Note that virtfs_proxy_helper does not work
> on macOS
> - Fully adjust meson virtfs error note to specify
> macOS
> - Rebase to master]
> Signed-off-by: Will Cohen <wwcohen@gmail.com>
> ---
> fsdev/meson.build | 1 +
> meson.build | 12 +++++++-----
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/fsdev/meson.build b/fsdev/meson.build
> index adf57cc43e..b632b66348 100644
> --- a/fsdev/meson.build
> +++ b/fsdev/meson.build
> @@ -7,6 +7,7 @@ fsdev_ss.add(when: ['CONFIG_FSDEV_9P'], if_true: files(
> 'qemu-fsdev.c',
> ), if_false: files('qemu-fsdev-dummy.c'))
> softmmu_ss.add_all(when: 'CONFIG_LINUX', if_true: fsdev_ss)
> +softmmu_ss.add_all(when: 'CONFIG_DARWIN', if_true: fsdev_ss)
>
> if have_virtfs_proxy_helper
> executable('virtfs-proxy-helper',
> diff --git a/meson.build b/meson.build
> index 3f8dca2c7a..ba52ed9e9a 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1450,14 +1450,16 @@ dbus_display = get_option('dbus_display') \
> .allowed()
>
> have_virtfs = get_option('virtfs') \
> - .require(targetos == 'linux',
> - error_message: 'virtio-9p (virtfs) requires Linux') \
> - .require(libattr.found() and libcap_ng.found(),
> - error_message: 'virtio-9p (virtfs) requires libcap-ng-devel and libattr-devel') \
> + .require(targetos == 'linux' or targetos == 'darwin',
> + error_message: 'virtio-9p (virtfs) requires Linux or macOS') \
> + .require(targetos == 'linux' or cc.has_function('pthread_fchdir_np'),
> + error_message: 'virtio-9p (virtfs) on macOS requires the presence of pthread_fchdir_np') \
> + .require(targetos == 'darwin' or (libattr.found() and libcap_ng.found()),
> + error_message: 'virtio-9p (virtfs) on Linux requires libcap-ng-devel and libattr-devel') \
> .disable_auto_if(not have_tools and not have_system) \
> .allowed()
>
> -have_virtfs_proxy_helper = have_virtfs and have_tools
> +have_virtfs_proxy_helper = targetos != 'darwin' and have_virtfs and have_tools
>
> foreach k : get_option('trace_backends')
> config_host_data.set('CONFIG_TRACE_' + k.to_upper(), true)
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat
2022-02-27 22:35 ` [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat Will Cohen
@ 2022-02-28 13:20 ` Christian Schoenebeck
2022-02-28 13:36 ` Thomas Huth
2022-02-28 13:37 ` Will Cohen
2022-04-08 13:52 ` Christian Schoenebeck
1 sibling, 2 replies; 27+ messages in thread
From: Christian Schoenebeck @ 2022-02-28 13:20 UTC (permalink / raw)
To: qemu-devel
Cc: Will Cohen, Laurent Vivier, Thomas Huth, Greg Kurz,
Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Paolo Bonzini, Keno Fischer
On Sonntag, 27. Februar 2022 23:35:20 CET Will Cohen wrote:
> From: Keno Fischer <keno@juliacomputing.com>
>
> Darwin does not support mknodat. However, to avoid race conditions
> with later setting the permissions, we must avoid using mknod on
> the full path instead. We could try to fchdir, but that would cause
> problems if multiple threads try to call mknodat at the same time.
> However, luckily there is a solution: Darwin includes a function
> that sets the cwd for the current thread only.
> This should suffice to use mknod safely.
>
> This function (pthread_fchdir_np) is protected by a check in
> meson in a patch later in this series.
>
> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
> [Will Cohen: - Adjust coding style
> - Replace clang references with gcc
> - Note radar filed with Apple for missing syscall
> - Replace direct syscall with pthread_fchdir_np and
> adjust patch notes accordingly
> - Declare pthread_fchdir_np with
> - __attribute__((weak_import)) to allow checking for
> its presence before usage
> - Move declarations above cplusplus guard
> - Add CONFIG_PTHREAD_FCHDIR_NP to meson and check for
> presence in 9p-util
> - Rebase to apply cleanly on top of the 2022-02-10
> changes to 9pfs
> - Fix line over 90 characters formatting error]
> Signed-off-by: Will Cohen <wwcohen@gmail.com>
> ---
> hw/9pfs/9p-local.c | 4 ++--
> hw/9pfs/9p-util-darwin.c | 33 +++++++++++++++++++++++++++++++++
> hw/9pfs/9p-util-linux.c | 6 ++++++
> hw/9pfs/9p-util.h | 11 +++++++++++
> meson.build | 1 +
> 5 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index a0d08e5216..d42ce6d8b8 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -682,7 +682,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath
> *dir_path,
>
> if (fs_ctx->export_flags & V9FS_SM_MAPPED ||
> fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
> - err = mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
> + err = qemu_mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
> if (err == -1) {
> goto out;
> }
> @@ -697,7 +697,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath
> *dir_path, }
> } else if (fs_ctx->export_flags & V9FS_SM_PASSTHROUGH ||
> fs_ctx->export_flags & V9FS_SM_NONE) {
> - err = mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
> + err = qemu_mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
> if (err == -1) {
> goto out;
> }
> diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> index cdb4c9e24c..bec0253474 100644
> --- a/hw/9pfs/9p-util-darwin.c
> +++ b/hw/9pfs/9p-util-darwin.c
> @@ -7,6 +7,8 @@
>
> #include "qemu/osdep.h"
> #include "qemu/xattr.h"
> +#include "qapi/error.h"
> +#include "qemu/error-report.h"
> #include "9p-util.h"
>
> ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char
> *name, @@ -62,3 +64,34 @@ int fsetxattrat_nofollow(int dirfd, const char
> *filename, const char *name, close_preserve_errno(fd);
> return ret;
> }
> +
> +/*
> + * As long as mknodat is not available on macOS, this workaround
> + * using pthread_fchdir_np is needed.
> + *
> + * Radar filed with Apple for implementing mknodat:
> + * rdar://FB9862426 (https://openradar.appspot.com/FB9862426)
> + */
> +#if defined CONFIG_PTHREAD_FCHDIR_NP
> +
> +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
> +{
> + int preserved_errno, err;
> + if (!pthread_fchdir_np) {
> + error_report_once("pthread_fchdir_np() not available on this version of macOS");
You took the code style error message a bit too literal; this line is still
too long:
WARNING: line over 80 characters
#199: FILE: hw/9pfs/9p-util-darwin.c:81:
+ error_report_once("pthread_fchdir_np() not available on this version of macOS");
total: 0 errors, 1 warnings, 91 lines checked
However this is too trivial to send a v10 just for this. If there are no other
issues in this v9 then I'll simply fix this on my end. My plan is to queue
this series tomorrow.
Best regards,
Christian Schoenebeck
> + return -ENOTSUP;
> + }
> + if (pthread_fchdir_np(dirfd) < 0) {
> + return -1;
> + }
> + err = mknod(filename, mode, dev);
> + preserved_errno = errno;
> + /* Stop using the thread-local cwd */
> + pthread_fchdir_np(-1);
> + if (err < 0) {
> + errno = preserved_errno;
> + }
> + return err;
> +}
> +
> +#endif
> diff --git a/hw/9pfs/9p-util-linux.c b/hw/9pfs/9p-util-linux.c
> index 398614a5d0..db451b0784 100644
> --- a/hw/9pfs/9p-util-linux.c
> +++ b/hw/9pfs/9p-util-linux.c
> @@ -61,4 +61,10 @@ int fsetxattrat_nofollow(int dirfd, const char *filename,
> const char *name, ret = lsetxattr(proc_path, name, value, size, flags);
> g_free(proc_path);
> 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 9abff79884..1f74d37558 100644
> --- a/hw/9pfs/9p-util.h
> +++ b/hw/9pfs/9p-util.h
> @@ -112,5 +112,16 @@ static inline off_t qemu_dirent_off(struct dirent
> *dent) #endif
> }
>
> +/*
> + * As long as mknodat is not available on macOS, this workaround
> + * using pthread_fchdir_np is needed. qemu_mknodat is defined in
> + * os-posix.c. pthread_fchdir_np is weakly linked here as a guard
> + * in case it disappears in future macOS versions, because it is
> + * is a private API.
> + */
> +#if defined CONFIG_DARWIN && defined CONFIG_PTHREAD_FCHDIR_NP
> +int pthread_fchdir_np(int fd) __attribute__((weak_import));
> +#endif
> +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev);
>
> #endif
> diff --git a/meson.build b/meson.build
> index 8df40bfac4..3f8dca2c7a 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1609,6 +1609,7 @@ config_host_data.set('CONFIG_POSIX_FALLOCATE',
> cc.has_function('posix_fallocate'
> config_host_data.set('CONFIG_POSIX_MEMALIGN',
> cc.has_function('posix_memalign')) config_host_data.set('CONFIG_PPOLL',
> cc.has_function('ppoll'))
> config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix:
> '#include <sys/uio.h>')) +config_host_data.set('CONFIG_PTHREAD_FCHDIR_NP',
> cc.has_function('pthread_fchdir_np'))
> config_host_data.set('CONFIG_SEM_TIMEDWAIT',
> cc.has_function('sem_timedwait', dependencies: threads))
> config_host_data.set('CONFIG_SENDFILE', cc.has_function('sendfile'))
> config_host_data.set('CONFIG_SETNS', cc.has_function('setns') and
> cc.has_function('unshare'))
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat
2022-02-28 13:20 ` Christian Schoenebeck
@ 2022-02-28 13:36 ` Thomas Huth
2022-02-28 13:51 ` Christian Schoenebeck
2022-02-28 13:37 ` Will Cohen
1 sibling, 1 reply; 27+ messages in thread
From: Thomas Huth @ 2022-02-28 13:36 UTC (permalink / raw)
To: Christian Schoenebeck, qemu-devel
Cc: Laurent Vivier, Greg Kurz, Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Will Cohen, Paolo Bonzini, Keno Fischer
On 28/02/2022 14.20, Christian Schoenebeck wrote:
> On Sonntag, 27. Februar 2022 23:35:20 CET Will Cohen wrote:
>> From: Keno Fischer <keno@juliacomputing.com>
>>
>> Darwin does not support mknodat. However, to avoid race conditions
>> with later setting the permissions, we must avoid using mknod on
>> the full path instead. We could try to fchdir, but that would cause
>> problems if multiple threads try to call mknodat at the same time.
>> However, luckily there is a solution: Darwin includes a function
>> that sets the cwd for the current thread only.
>> This should suffice to use mknod safely.
>>
>> This function (pthread_fchdir_np) is protected by a check in
>> meson in a patch later in this series.
>>
>> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
>> Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
>> [Will Cohen: - Adjust coding style
>> - Replace clang references with gcc
>> - Note radar filed with Apple for missing syscall
>> - Replace direct syscall with pthread_fchdir_np and
>> adjust patch notes accordingly
>> - Declare pthread_fchdir_np with
>> - __attribute__((weak_import)) to allow checking for
>> its presence before usage
>> - Move declarations above cplusplus guard
>> - Add CONFIG_PTHREAD_FCHDIR_NP to meson and check for
>> presence in 9p-util
>> - Rebase to apply cleanly on top of the 2022-02-10
>> changes to 9pfs
>> - Fix line over 90 characters formatting error]
>> Signed-off-by: Will Cohen <wwcohen@gmail.com>
>> ---
>> hw/9pfs/9p-local.c | 4 ++--
>> hw/9pfs/9p-util-darwin.c | 33 +++++++++++++++++++++++++++++++++
>> hw/9pfs/9p-util-linux.c | 6 ++++++
>> hw/9pfs/9p-util.h | 11 +++++++++++
>> meson.build | 1 +
>> 5 files changed, 53 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
>> index a0d08e5216..d42ce6d8b8 100644
>> --- a/hw/9pfs/9p-local.c
>> +++ b/hw/9pfs/9p-local.c
>> @@ -682,7 +682,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath
>> *dir_path,
>>
>> if (fs_ctx->export_flags & V9FS_SM_MAPPED ||
>> fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
>> - err = mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
>> + err = qemu_mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
>> if (err == -1) {
>> goto out;
>> }
>> @@ -697,7 +697,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath
>> *dir_path, }
>> } else if (fs_ctx->export_flags & V9FS_SM_PASSTHROUGH ||
>> fs_ctx->export_flags & V9FS_SM_NONE) {
>> - err = mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
>> + err = qemu_mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
>> if (err == -1) {
>> goto out;
>> }
>> diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
>> index cdb4c9e24c..bec0253474 100644
>> --- a/hw/9pfs/9p-util-darwin.c
>> +++ b/hw/9pfs/9p-util-darwin.c
>> @@ -7,6 +7,8 @@
>>
>> #include "qemu/osdep.h"
>> #include "qemu/xattr.h"
>> +#include "qapi/error.h"
>> +#include "qemu/error-report.h"
>> #include "9p-util.h"
>>
>> ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char
>> *name, @@ -62,3 +64,34 @@ int fsetxattrat_nofollow(int dirfd, const char
>> *filename, const char *name, close_preserve_errno(fd);
>> return ret;
>> }
>> +
>> +/*
>> + * As long as mknodat is not available on macOS, this workaround
>> + * using pthread_fchdir_np is needed.
>> + *
>> + * Radar filed with Apple for implementing mknodat:
>> + * rdar://FB9862426 (https://openradar.appspot.com/FB9862426)
>> + */
>> +#if defined CONFIG_PTHREAD_FCHDIR_NP
>> +
>> +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
>> +{
>> + int preserved_errno, err;
>> + if (!pthread_fchdir_np) {
>> + error_report_once("pthread_fchdir_np() not available on this version of macOS");
>
> You took the code style error message a bit too literal; this line is still
> too long:
>
> WARNING: line over 80 characters
> #199: FILE: hw/9pfs/9p-util-darwin.c:81:
> + error_report_once("pthread_fchdir_np() not available on this version of macOS");
>
> total: 0 errors, 1 warnings, 91 lines checked
>
> However this is too trivial to send a v10 just for this. If there are no other
> issues in this v9 then I'll simply fix this on my end. My plan is to queue
> this series tomorrow.
For lines less than 90 characters, it's just a warning, and I think it's ok
in such cases to keep it longer than 80 characters, if the result of
breaking it up would look more awkward otherwise.
Thomas
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat
2022-02-28 13:20 ` Christian Schoenebeck
2022-02-28 13:36 ` Thomas Huth
@ 2022-02-28 13:37 ` Will Cohen
2022-02-28 13:41 ` Greg Kurz
1 sibling, 1 reply; 27+ messages in thread
From: Will Cohen @ 2022-02-28 13:37 UTC (permalink / raw)
To: Christian Schoenebeck
Cc: Laurent Vivier, Thomas Huth, qemu-devel, Greg Kurz,
Philippe Mathieu-Daudé,
Keno Fischer, Michael Roitzsch, Paolo Bonzini, hi
[-- Attachment #1: Type: text/plain, Size: 7614 bytes --]
On Mon, Feb 28, 2022 at 8:20 AM Christian Schoenebeck <
qemu_oss@crudebyte.com> wrote:
> On Sonntag, 27. Februar 2022 23:35:20 CET Will Cohen wrote:
> > From: Keno Fischer <keno@juliacomputing.com>
> >
> > Darwin does not support mknodat. However, to avoid race conditions
> > with later setting the permissions, we must avoid using mknod on
> > the full path instead. We could try to fchdir, but that would cause
> > problems if multiple threads try to call mknodat at the same time.
> > However, luckily there is a solution: Darwin includes a function
> > that sets the cwd for the current thread only.
> > This should suffice to use mknod safely.
> >
> > This function (pthread_fchdir_np) is protected by a check in
> > meson in a patch later in this series.
> >
> > Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> > Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
> > [Will Cohen: - Adjust coding style
> > - Replace clang references with gcc
> > - Note radar filed with Apple for missing syscall
> > - Replace direct syscall with pthread_fchdir_np and
> > adjust patch notes accordingly
> > - Declare pthread_fchdir_np with
> > - __attribute__((weak_import)) to allow checking for
> > its presence before usage
> > - Move declarations above cplusplus guard
> > - Add CONFIG_PTHREAD_FCHDIR_NP to meson and check for
> > presence in 9p-util
> > - Rebase to apply cleanly on top of the 2022-02-10
> > changes to 9pfs
> > - Fix line over 90 characters formatting error]
> > Signed-off-by: Will Cohen <wwcohen@gmail.com>
> > ---
> > hw/9pfs/9p-local.c | 4 ++--
> > hw/9pfs/9p-util-darwin.c | 33 +++++++++++++++++++++++++++++++++
> > hw/9pfs/9p-util-linux.c | 6 ++++++
> > hw/9pfs/9p-util.h | 11 +++++++++++
> > meson.build | 1 +
> > 5 files changed, 53 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> > index a0d08e5216..d42ce6d8b8 100644
> > --- a/hw/9pfs/9p-local.c
> > +++ b/hw/9pfs/9p-local.c
> > @@ -682,7 +682,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath
> > *dir_path,
> >
> > if (fs_ctx->export_flags & V9FS_SM_MAPPED ||
> > fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
> > - err = mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
> > + err = qemu_mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
> > if (err == -1) {
> > goto out;
> > }
> > @@ -697,7 +697,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath
> > *dir_path, }
> > } else if (fs_ctx->export_flags & V9FS_SM_PASSTHROUGH ||
> > fs_ctx->export_flags & V9FS_SM_NONE) {
> > - err = mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
> > + err = qemu_mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
> > if (err == -1) {
> > goto out;
> > }
> > diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> > index cdb4c9e24c..bec0253474 100644
> > --- a/hw/9pfs/9p-util-darwin.c
> > +++ b/hw/9pfs/9p-util-darwin.c
> > @@ -7,6 +7,8 @@
> >
> > #include "qemu/osdep.h"
> > #include "qemu/xattr.h"
> > +#include "qapi/error.h"
> > +#include "qemu/error-report.h"
> > #include "9p-util.h"
> >
> > ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char
> > *name, @@ -62,3 +64,34 @@ int fsetxattrat_nofollow(int dirfd, const char
> > *filename, const char *name, close_preserve_errno(fd);
> > return ret;
> > }
> > +
> > +/*
> > + * As long as mknodat is not available on macOS, this workaround
> > + * using pthread_fchdir_np is needed.
> > + *
> > + * Radar filed with Apple for implementing mknodat:
> > + * rdar://FB9862426 (https://openradar.appspot.com/FB9862426)
> > + */
> > +#if defined CONFIG_PTHREAD_FCHDIR_NP
> > +
> > +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t
> dev)
> > +{
> > + int preserved_errno, err;
> > + if (!pthread_fchdir_np) {
> > + error_report_once("pthread_fchdir_np() not available on this
> version of macOS");
>
> You took the code style error message a bit too literal; this line is still
> too long:
>
> WARNING: line over 80 characters
> #199: FILE: hw/9pfs/9p-util-darwin.c:81:
> + error_report_once("pthread_fchdir_np() not available on this
> version of macOS");
>
> total: 0 errors, 1 warnings, 91 lines checked
>
> However this is too trivial to send a v10 just for this. If there are no
> other
> issues in this v9 then I'll simply fix this on my end. My plan is to queue
> this series tomorrow.
>
> Best regards,
> Christian Schoenebeck
Sorry for the over-literalness. I was just trying to avoid a need for
further changes by being too dramatic on my end for a small fix! Any
stylistic changes needed here are, of course, totally acceptable!
>
>
> > + return -ENOTSUP;
> > + }
> > + if (pthread_fchdir_np(dirfd) < 0) {
> > + return -1;
> > + }
> > + err = mknod(filename, mode, dev);
> > + preserved_errno = errno;
> > + /* Stop using the thread-local cwd */
> > + pthread_fchdir_np(-1);
> > + if (err < 0) {
> > + errno = preserved_errno;
> > + }
> > + return err;
> > +}
> > +
> > +#endif
> > diff --git a/hw/9pfs/9p-util-linux.c b/hw/9pfs/9p-util-linux.c
> > index 398614a5d0..db451b0784 100644
> > --- a/hw/9pfs/9p-util-linux.c
> > +++ b/hw/9pfs/9p-util-linux.c
> > @@ -61,4 +61,10 @@ int fsetxattrat_nofollow(int dirfd, const char
> *filename,
> > const char *name, ret = lsetxattr(proc_path, name, value, size, flags);
> > g_free(proc_path);
> > 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 9abff79884..1f74d37558 100644
> > --- a/hw/9pfs/9p-util.h
> > +++ b/hw/9pfs/9p-util.h
> > @@ -112,5 +112,16 @@ static inline off_t qemu_dirent_off(struct dirent
> > *dent) #endif
> > }
> >
> > +/*
> > + * As long as mknodat is not available on macOS, this workaround
> > + * using pthread_fchdir_np is needed. qemu_mknodat is defined in
> > + * os-posix.c. pthread_fchdir_np is weakly linked here as a guard
> > + * in case it disappears in future macOS versions, because it is
> > + * is a private API.
> > + */
> > +#if defined CONFIG_DARWIN && defined CONFIG_PTHREAD_FCHDIR_NP
> > +int pthread_fchdir_np(int fd) __attribute__((weak_import));
> > +#endif
> > +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t
> dev);
> >
> > #endif
> > diff --git a/meson.build b/meson.build
> > index 8df40bfac4..3f8dca2c7a 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -1609,6 +1609,7 @@ config_host_data.set('CONFIG_POSIX_FALLOCATE',
> > cc.has_function('posix_fallocate'
> > config_host_data.set('CONFIG_POSIX_MEMALIGN',
> > cc.has_function('posix_memalign')) config_host_data.set('CONFIG_PPOLL',
> > cc.has_function('ppoll'))
> > config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix:
> > '#include <sys/uio.h>'))
> +config_host_data.set('CONFIG_PTHREAD_FCHDIR_NP',
> > cc.has_function('pthread_fchdir_np'))
> > config_host_data.set('CONFIG_SEM_TIMEDWAIT',
> > cc.has_function('sem_timedwait', dependencies: threads))
> > config_host_data.set('CONFIG_SENDFILE', cc.has_function('sendfile'))
> > config_host_data.set('CONFIG_SETNS', cc.has_function('setns') and
> > cc.has_function('unshare'))
>
>
>
[-- Attachment #2: Type: text/html, Size: 10074 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat
2022-02-28 13:37 ` Will Cohen
@ 2022-02-28 13:41 ` Greg Kurz
0 siblings, 0 replies; 27+ messages in thread
From: Greg Kurz @ 2022-02-28 13:41 UTC (permalink / raw)
To: Will Cohen
Cc: Laurent Vivier, Thomas Huth, Christian Schoenebeck,
Philippe Mathieu-Daudé,
qemu-devel, Keno Fischer, Michael Roitzsch, Paolo Bonzini, hi
On Mon, 28 Feb 2022 08:37:10 -0500
Will Cohen <wwcohen@gmail.com> wrote:
> On Mon, Feb 28, 2022 at 8:20 AM Christian Schoenebeck <
> qemu_oss@crudebyte.com> wrote:
>
> > On Sonntag, 27. Februar 2022 23:35:20 CET Will Cohen wrote:
> > > From: Keno Fischer <keno@juliacomputing.com>
> > >
> > > Darwin does not support mknodat. However, to avoid race conditions
> > > with later setting the permissions, we must avoid using mknod on
> > > the full path instead. We could try to fchdir, but that would cause
> > > problems if multiple threads try to call mknodat at the same time.
> > > However, luckily there is a solution: Darwin includes a function
> > > that sets the cwd for the current thread only.
> > > This should suffice to use mknod safely.
> > >
> > > This function (pthread_fchdir_np) is protected by a check in
> > > meson in a patch later in this series.
> > >
> > > Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> > > Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
> > > [Will Cohen: - Adjust coding style
> > > - Replace clang references with gcc
> > > - Note radar filed with Apple for missing syscall
> > > - Replace direct syscall with pthread_fchdir_np and
> > > adjust patch notes accordingly
> > > - Declare pthread_fchdir_np with
> > > - __attribute__((weak_import)) to allow checking for
> > > its presence before usage
> > > - Move declarations above cplusplus guard
> > > - Add CONFIG_PTHREAD_FCHDIR_NP to meson and check for
> > > presence in 9p-util
> > > - Rebase to apply cleanly on top of the 2022-02-10
> > > changes to 9pfs
> > > - Fix line over 90 characters formatting error]
> > > Signed-off-by: Will Cohen <wwcohen@gmail.com>
> > > ---
> > > hw/9pfs/9p-local.c | 4 ++--
> > > hw/9pfs/9p-util-darwin.c | 33 +++++++++++++++++++++++++++++++++
> > > hw/9pfs/9p-util-linux.c | 6 ++++++
> > > hw/9pfs/9p-util.h | 11 +++++++++++
> > > meson.build | 1 +
> > > 5 files changed, 53 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> > > index a0d08e5216..d42ce6d8b8 100644
> > > --- a/hw/9pfs/9p-local.c
> > > +++ b/hw/9pfs/9p-local.c
> > > @@ -682,7 +682,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath
> > > *dir_path,
> > >
> > > if (fs_ctx->export_flags & V9FS_SM_MAPPED ||
> > > fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
> > > - err = mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
> > > + err = qemu_mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
> > > if (err == -1) {
> > > goto out;
> > > }
> > > @@ -697,7 +697,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath
> > > *dir_path, }
> > > } else if (fs_ctx->export_flags & V9FS_SM_PASSTHROUGH ||
> > > fs_ctx->export_flags & V9FS_SM_NONE) {
> > > - err = mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
> > > + err = qemu_mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
> > > if (err == -1) {
> > > goto out;
> > > }
> > > diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> > > index cdb4c9e24c..bec0253474 100644
> > > --- a/hw/9pfs/9p-util-darwin.c
> > > +++ b/hw/9pfs/9p-util-darwin.c
> > > @@ -7,6 +7,8 @@
> > >
> > > #include "qemu/osdep.h"
> > > #include "qemu/xattr.h"
> > > +#include "qapi/error.h"
> > > +#include "qemu/error-report.h"
> > > #include "9p-util.h"
> > >
> > > ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char
> > > *name, @@ -62,3 +64,34 @@ int fsetxattrat_nofollow(int dirfd, const char
> > > *filename, const char *name, close_preserve_errno(fd);
> > > return ret;
> > > }
> > > +
> > > +/*
> > > + * As long as mknodat is not available on macOS, this workaround
> > > + * using pthread_fchdir_np is needed.
> > > + *
> > > + * Radar filed with Apple for implementing mknodat:
> > > + * rdar://FB9862426 (https://openradar.appspot.com/FB9862426)
> > > + */
> > > +#if defined CONFIG_PTHREAD_FCHDIR_NP
> > > +
> > > +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t
> > dev)
> > > +{
> > > + int preserved_errno, err;
> > > + if (!pthread_fchdir_np) {
> > > + error_report_once("pthread_fchdir_np() not available on this
> > version of macOS");
> >
> > You took the code style error message a bit too literal; this line is still
> > too long:
> >
> > WARNING: line over 80 characters
> > #199: FILE: hw/9pfs/9p-util-darwin.c:81:
> > + error_report_once("pthread_fchdir_np() not available on this
> > version of macOS");
> >
> > total: 0 errors, 1 warnings, 91 lines checked
> >
> > However this is too trivial to send a v10 just for this. If there are no
> > other
> > issues in this v9 then I'll simply fix this on my end. My plan is to queue
> > this series tomorrow.
> >
> > Best regards,
> > Christian Schoenebeck
>
>
> Sorry for the over-literalness. I was just trying to avoid a need for
> further changes by being too dramatic on my end for a small fix! Any
> stylistic changes needed here are, of course, totally acceptable!
>
As Thomas is saying this is just a warning and it is certainly
easier for a developer to grep the error message if it is kept
on a single line.
> >
> >
> > > + return -ENOTSUP;
> > > + }
> > > + if (pthread_fchdir_np(dirfd) < 0) {
> > > + return -1;
> > > + }
> > > + err = mknod(filename, mode, dev);
> > > + preserved_errno = errno;
> > > + /* Stop using the thread-local cwd */
> > > + pthread_fchdir_np(-1);
> > > + if (err < 0) {
> > > + errno = preserved_errno;
> > > + }
> > > + return err;
> > > +}
> > > +
> > > +#endif
> > > diff --git a/hw/9pfs/9p-util-linux.c b/hw/9pfs/9p-util-linux.c
> > > index 398614a5d0..db451b0784 100644
> > > --- a/hw/9pfs/9p-util-linux.c
> > > +++ b/hw/9pfs/9p-util-linux.c
> > > @@ -61,4 +61,10 @@ int fsetxattrat_nofollow(int dirfd, const char
> > *filename,
> > > const char *name, ret = lsetxattr(proc_path, name, value, size, flags);
> > > g_free(proc_path);
> > > 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 9abff79884..1f74d37558 100644
> > > --- a/hw/9pfs/9p-util.h
> > > +++ b/hw/9pfs/9p-util.h
> > > @@ -112,5 +112,16 @@ static inline off_t qemu_dirent_off(struct dirent
> > > *dent) #endif
> > > }
> > >
> > > +/*
> > > + * As long as mknodat is not available on macOS, this workaround
> > > + * using pthread_fchdir_np is needed. qemu_mknodat is defined in
> > > + * os-posix.c. pthread_fchdir_np is weakly linked here as a guard
> > > + * in case it disappears in future macOS versions, because it is
> > > + * is a private API.
> > > + */
> > > +#if defined CONFIG_DARWIN && defined CONFIG_PTHREAD_FCHDIR_NP
> > > +int pthread_fchdir_np(int fd) __attribute__((weak_import));
> > > +#endif
> > > +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t
> > dev);
> > >
> > > #endif
> > > diff --git a/meson.build b/meson.build
> > > index 8df40bfac4..3f8dca2c7a 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -1609,6 +1609,7 @@ config_host_data.set('CONFIG_POSIX_FALLOCATE',
> > > cc.has_function('posix_fallocate'
> > > config_host_data.set('CONFIG_POSIX_MEMALIGN',
> > > cc.has_function('posix_memalign')) config_host_data.set('CONFIG_PPOLL',
> > > cc.has_function('ppoll'))
> > > config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix:
> > > '#include <sys/uio.h>'))
> > +config_host_data.set('CONFIG_PTHREAD_FCHDIR_NP',
> > > cc.has_function('pthread_fchdir_np'))
> > > config_host_data.set('CONFIG_SEM_TIMEDWAIT',
> > > cc.has_function('sem_timedwait', dependencies: threads))
> > > config_host_data.set('CONFIG_SENDFILE', cc.has_function('sendfile'))
> > > config_host_data.set('CONFIG_SETNS', cc.has_function('setns') and
> > > cc.has_function('unshare'))
> >
> >
> >
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v9 11/11] 9p: darwin: meson: Allow VirtFS on Darwin
2022-02-28 13:11 ` Christian Schoenebeck
@ 2022-02-28 13:43 ` Will Cohen
0 siblings, 0 replies; 27+ messages in thread
From: Will Cohen @ 2022-02-28 13:43 UTC (permalink / raw)
To: Christian Schoenebeck
Cc: Laurent Vivier, Thomas Huth, qemu-devel, Greg Kurz,
Philippe Mathieu-Daudé,
Keno Fischer, Michael Roitzsch, Paolo Bonzini, hi
[-- Attachment #1: Type: text/plain, Size: 3663 bytes --]
On Mon, Feb 28, 2022 at 8:11 AM Christian Schoenebeck <
qemu_oss@crudebyte.com> wrote:
> On Sonntag, 27. Februar 2022 23:35:22 CET Will Cohen wrote:
> > From: Keno Fischer <keno@juliacomputing.com>
> >
> > To allow VirtFS on darwin, we need to check that pthread_fchdir_np is
> > available, which has only been available since macOS 10.12.
> >
> > Additionally, virtfs_proxy_helper is disabled on Darwin. This patch
> > series does not currently provide an implementation of the proxy-helper,
> > but this functionality could be implemented later on.
> >
> > Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> > [Michael Roitzsch: - Rebase for NixOS]
> > Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
> > [Will Cohen: - Rebase to master]
> > Signed-off-by: Will Cohen <wwcohen@gmail.com>
> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Paolo, could you have a look at this patch, please? It has changed quite a
> bit
> since your review.
>
> Best regards,
> Christian Schoenebeck
Yes. I probably should have noted that in the cover letter. The code style
itself totally changed in this last version as well, but now conforms to
the other changes made to HEAD during this last rebase.
>
> > [Will Cohen: - Add check for pthread_fchdir_np to virtfs
> > - Add comments to patch commit
> > - Note that virtfs_proxy_helper does not work
> > on macOS
> > - Fully adjust meson virtfs error note to specify
> > macOS
> > - Rebase to master]
> > Signed-off-by: Will Cohen <wwcohen@gmail.com>
> > ---
> > fsdev/meson.build | 1 +
> > meson.build | 12 +++++++-----
> > 2 files changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/fsdev/meson.build b/fsdev/meson.build
> > index adf57cc43e..b632b66348 100644
> > --- a/fsdev/meson.build
> > +++ b/fsdev/meson.build
> > @@ -7,6 +7,7 @@ fsdev_ss.add(when: ['CONFIG_FSDEV_9P'], if_true: files(
> > 'qemu-fsdev.c',
> > ), if_false: files('qemu-fsdev-dummy.c'))
> > softmmu_ss.add_all(when: 'CONFIG_LINUX', if_true: fsdev_ss)
> > +softmmu_ss.add_all(when: 'CONFIG_DARWIN', if_true: fsdev_ss)
> >
> > if have_virtfs_proxy_helper
> > executable('virtfs-proxy-helper',
> > diff --git a/meson.build b/meson.build
> > index 3f8dca2c7a..ba52ed9e9a 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -1450,14 +1450,16 @@ dbus_display = get_option('dbus_display') \
> > .allowed()
> >
> > have_virtfs = get_option('virtfs') \
> > - .require(targetos == 'linux',
> > - error_message: 'virtio-9p (virtfs) requires Linux') \
> > - .require(libattr.found() and libcap_ng.found(),
> > - error_message: 'virtio-9p (virtfs) requires
> libcap-ng-devel and libattr-devel') \
> > + .require(targetos == 'linux' or targetos == 'darwin',
> > + error_message: 'virtio-9p (virtfs) requires Linux or
> macOS') \
> > + .require(targetos == 'linux' or
> cc.has_function('pthread_fchdir_np'),
> > + error_message: 'virtio-9p (virtfs) on macOS requires the
> presence of pthread_fchdir_np') \
> > + .require(targetos == 'darwin' or (libattr.found() and
> libcap_ng.found()),
> > + error_message: 'virtio-9p (virtfs) on Linux requires
> libcap-ng-devel and libattr-devel') \
> > .disable_auto_if(not have_tools and not have_system) \
> > .allowed()
> >
> > -have_virtfs_proxy_helper = have_virtfs and have_tools
> > +have_virtfs_proxy_helper = targetos != 'darwin' and have_virtfs and
> have_tools
> >
> > foreach k : get_option('trace_backends')
> > config_host_data.set('CONFIG_TRACE_' + k.to_upper(), true)
>
>
>
[-- Attachment #2: Type: text/html, Size: 5302 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat
2022-02-28 13:36 ` Thomas Huth
@ 2022-02-28 13:51 ` Christian Schoenebeck
2022-02-28 14:06 ` Peter Maydell
0 siblings, 1 reply; 27+ messages in thread
From: Christian Schoenebeck @ 2022-02-28 13:51 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Laurent Vivier, Greg Kurz,
Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Will Cohen, Paolo Bonzini, Keno Fischer
On Montag, 28. Februar 2022 14:36:30 CET Thomas Huth wrote:
> On 28/02/2022 14.20, Christian Schoenebeck wrote:
> > On Sonntag, 27. Februar 2022 23:35:20 CET Will Cohen wrote:
> >> From: Keno Fischer <keno@juliacomputing.com>
> >>
> >> Darwin does not support mknodat. However, to avoid race conditions
> >> with later setting the permissions, we must avoid using mknod on
> >> the full path instead. We could try to fchdir, but that would cause
> >> problems if multiple threads try to call mknodat at the same time.
> >> However, luckily there is a solution: Darwin includes a function
> >> that sets the cwd for the current thread only.
> >> This should suffice to use mknod safely.
> >>
> >> This function (pthread_fchdir_np) is protected by a check in
> >> meson in a patch later in this series.
> >>
> >> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> >> Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
> >> [Will Cohen: - Adjust coding style
> >>
> >> - Replace clang references with gcc
> >> - Note radar filed with Apple for missing syscall
> >> - Replace direct syscall with pthread_fchdir_np and
> >>
> >> adjust patch notes accordingly
> >>
> >> - Declare pthread_fchdir_np with
> >> - __attribute__((weak_import)) to allow checking for
> >>
> >> its presence before usage
> >>
> >> - Move declarations above cplusplus guard
> >> - Add CONFIG_PTHREAD_FCHDIR_NP to meson and check for
> >>
> >> presence in 9p-util
> >>
> >> - Rebase to apply cleanly on top of the 2022-02-10
> >>
> >> changes to 9pfs
> >>
> >> - Fix line over 90 characters formatting error]
> >>
> >> Signed-off-by: Will Cohen <wwcohen@gmail.com>
> >> ---
> >>
> >> hw/9pfs/9p-local.c | 4 ++--
> >> hw/9pfs/9p-util-darwin.c | 33 +++++++++++++++++++++++++++++++++
> >> hw/9pfs/9p-util-linux.c | 6 ++++++
> >> hw/9pfs/9p-util.h | 11 +++++++++++
> >> meson.build | 1 +
> >> 5 files changed, 53 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> >> index a0d08e5216..d42ce6d8b8 100644
> >> --- a/hw/9pfs/9p-local.c
> >> +++ b/hw/9pfs/9p-local.c
> >> @@ -682,7 +682,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath
> >> *dir_path,
> >>
> >> if (fs_ctx->export_flags & V9FS_SM_MAPPED ||
> >>
> >> fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
> >>
> >> - err = mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
> >> + err = qemu_mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
> >>
> >> if (err == -1) {
> >>
> >> goto out;
> >>
> >> }
> >>
> >> @@ -697,7 +697,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath
> >> *dir_path, }
> >>
> >> } else if (fs_ctx->export_flags & V9FS_SM_PASSTHROUGH ||
> >>
> >> fs_ctx->export_flags & V9FS_SM_NONE) {
> >>
> >> - err = mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
> >> + err = qemu_mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
> >>
> >> if (err == -1) {
> >>
> >> goto out;
> >>
> >> }
> >>
> >> diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> >> index cdb4c9e24c..bec0253474 100644
> >> --- a/hw/9pfs/9p-util-darwin.c
> >> +++ b/hw/9pfs/9p-util-darwin.c
> >> @@ -7,6 +7,8 @@
> >>
> >> #include "qemu/osdep.h"
> >> #include "qemu/xattr.h"
> >>
> >> +#include "qapi/error.h"
> >> +#include "qemu/error-report.h"
> >>
> >> #include "9p-util.h"
> >>
> >> ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const
> >> char
> >>
> >> *name, @@ -62,3 +64,34 @@ int fsetxattrat_nofollow(int dirfd, const char
> >> *filename, const char *name, close_preserve_errno(fd);
> >>
> >> return ret;
> >>
> >> }
> >>
> >> +
> >> +/*
> >> + * As long as mknodat is not available on macOS, this workaround
> >> + * using pthread_fchdir_np is needed.
> >> + *
> >> + * Radar filed with Apple for implementing mknodat:
> >> + * rdar://FB9862426 (https://openradar.appspot.com/FB9862426)
> >> + */
> >> +#if defined CONFIG_PTHREAD_FCHDIR_NP
> >> +
> >> +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t
> >> dev)
> >> +{
> >> + int preserved_errno, err;
> >> + if (!pthread_fchdir_np) {
> >> + error_report_once("pthread_fchdir_np() not available on this
> >> version of macOS");>
> > You took the code style error message a bit too literal; this line is
> > still
> > too long:
> >
> > WARNING: line over 80 characters
> > #199: FILE: hw/9pfs/9p-util-darwin.c:81:
> > + error_report_once("pthread_fchdir_np() not available on this
> > version of macOS");
> >
> > total: 0 errors, 1 warnings, 91 lines checked
> >
> > However this is too trivial to send a v10 just for this. If there are no
> > other issues in this v9 then I'll simply fix this on my end. My plan is
> > to queue this series tomorrow.
>
> For lines less than 90 characters, it's just a warning, and I think it's ok
> in such cases to keep it longer than 80 characters, if the result of
> breaking it up would look more awkward otherwise.
>
> Thomas
This doesn't look awkward to me:
error_report_once(
"pthread_fchdir_np() is not available on this version of macOS"
);
It silences the warning and grep is not affected either.
Best regards,
Christian Schoenebeck
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat
2022-02-28 13:51 ` Christian Schoenebeck
@ 2022-02-28 14:06 ` Peter Maydell
2022-02-28 15:45 ` Christian Schoenebeck
0 siblings, 1 reply; 27+ messages in thread
From: Peter Maydell @ 2022-02-28 14:06 UTC (permalink / raw)
To: Christian Schoenebeck
Cc: Laurent Vivier, Thomas Huth, Philippe Mathieu-Daudé,
qemu-devel, hi, Michael Roitzsch, Will Cohen, Paolo Bonzini,
Keno Fischer, Greg Kurz
On Mon, 28 Feb 2022 at 13:58, Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
>
> On Montag, 28. Februar 2022 14:36:30 CET Thomas Huth wrote:
> > For lines less than 90 characters, it's just a warning, and I think it's ok
> > in such cases to keep it longer than 80 characters, if the result of
> > breaking it up would look more awkward otherwise.
> >
> > Thomas
>
> This doesn't look awkward to me:
>
> error_report_once(
> "pthread_fchdir_np() is not available on this version of macOS"
> );
I think that looks pretty strange, though "git grep -A3 -- '($'" does show
other examples of doing it that way. I'd favour leaving it as a single
line, which the style guide allows ("better to have an 85 character line
than one which is awkwardly wrapped").
Personally I would favour just not warning at all about the more-than-80
less-than-90 lines case: it mostly tends to produce discussions like this
one and people preferring to break lines that would be better unbroken.
I know not everybody agrees with that, though.
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat
2022-02-28 14:06 ` Peter Maydell
@ 2022-02-28 15:45 ` Christian Schoenebeck
0 siblings, 0 replies; 27+ messages in thread
From: Christian Schoenebeck @ 2022-02-28 15:45 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Laurent Vivier, Thomas Huth,
Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Will Cohen, Paolo Bonzini, Keno Fischer,
Greg Kurz
On Montag, 28. Februar 2022 15:06:07 CET Peter Maydell wrote:
> On Mon, 28 Feb 2022 at 13:58, Christian Schoenebeck
>
> <qemu_oss@crudebyte.com> wrote:
> > On Montag, 28. Februar 2022 14:36:30 CET Thomas Huth wrote:
> > > For lines less than 90 characters, it's just a warning, and I think it's
> > > ok
> > > in such cases to keep it longer than 80 characters, if the result of
> > > breaking it up would look more awkward otherwise.
> > >
> > > Thomas
> >
> > This doesn't look awkward to me:
> > error_report_once(
> >
> > "pthread_fchdir_np() is not available on this version of
> > macOS"
> >
> > );
>
> I think that looks pretty strange, though "git grep -A3 -- '($'" does show
> other examples of doing it that way. I'd favour leaving it as a single
> line, which the style guide allows ("better to have an 85 character line
> than one which is awkwardly wrapped").
>
> Personally I would favour just not warning at all about the more-than-80
> less-than-90 lines case: it mostly tends to produce discussions like this
> one and people preferring to break lines that would be better unbroken.
> I know not everybody agrees with that, though.
>
> -- PMM
There is a practical reason for keeping things <80 chars: some email clients
like mine do awkward attempts to constrain lines to 80 chars in replies, which
I then always have to manually fix for the quoted diff being readable again.
E.g. in this case it screwed it like this:
> +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
> +{
> + int preserved_errno, err;
> + if (!pthread_fchdir_np) {
> + error_report_once("pthread_fchdir_np() not available on this
> version of macOS"); + return -ENOTSUP;
> + }
> + if (pthread_fchdir_np(dirfd) < 0) {
> + return -1;
> + }
Anyway, I leave this as-is then, as I seem to have a minority opinion.
Best regards,
Christian Schoenebeck
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v9 00/11] 9p: Add support for darwin
2022-02-27 22:35 [PATCH v9 00/11] 9p: Add support for darwin Will Cohen
` (10 preceding siblings ...)
2022-02-27 22:35 ` [PATCH v9 11/11] 9p: darwin: meson: Allow VirtFS on Darwin Will Cohen
@ 2022-03-01 19:25 ` Christian Schoenebeck
2022-03-01 20:09 ` Will Cohen
11 siblings, 1 reply; 27+ messages in thread
From: Christian Schoenebeck @ 2022-03-01 19:25 UTC (permalink / raw)
To: qemu-devel
Cc: Will Cohen, Laurent Vivier, Thomas Huth, Greg Kurz,
Philippe Mathieu-Daudé,
hi, Paolo Bonzini
On Sonntag, 27. Februar 2022 23:35:11 CET Will Cohen wrote:
> This is a followup to
> https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg04391.html, adding
> 9p server support for Darwin.
>
> Since v8, the following changes have been made:
>
> Patch 4/11 (9p: darwin: Handle struct dirent differences)
> - Declare qemu_dirent_off as static to prevent linker error
> - Move qemu_dirent_off above the end-of-file endif to fix compilation
>
> Patch 9/11 (9p: darwin: Implement compatibility for mknodat)
> - Fix line over 90 characters formatting error
> - Move qemu_mknodat back from osdep to 9p-util and adjust patch notes
> accordingly
>
> Patch 11/11 (9p: darwin: meson: Allow VirtFS on Darwin)
> - Rebase to master
>
> With these changes, this patch set builds and passes 9p synth tests on both
> linux and darwin.
>
> Keno Fischer (10):
> 9p: linux: Fix a couple Linux assumptions
> 9p: Rename 9p-util -> 9p-util-linux
> 9p: darwin: Handle struct stat(fs) differences
> 9p: darwin: Handle struct dirent differences
> 9p: darwin: Ignore O_{NOATIME, DIRECT}
> 9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX
> 9p: darwin: *xattr_nofollow implementations
> 9p: darwin: Compatibility for f/l*xattr
> 9p: darwin: Implement compatibility for mknodat
> 9p: darwin: meson: Allow VirtFS on Darwin
>
> Will Cohen (1):
> 9p: darwin: Adjust assumption on virtio-9p-test
>
> fsdev/file-op-9p.h | 9 ++-
> fsdev/meson.build | 1 +
> hw/9pfs/9p-local.c | 27 +++++--
> hw/9pfs/9p-proxy.c | 38 +++++++++-
> hw/9pfs/9p-synth.c | 6 ++
> hw/9pfs/9p-util-darwin.c | 97 ++++++++++++++++++++++++++
> hw/9pfs/{9p-util.c => 9p-util-linux.c} | 8 ++-
> hw/9pfs/9p-util.h | 46 ++++++++++++
> hw/9pfs/9p.c | 42 +++++++++--
> hw/9pfs/9p.h | 18 +++++
> hw/9pfs/codir.c | 4 +-
> hw/9pfs/meson.build | 3 +-
> include/qemu/xattr.h | 4 +-
> meson.build | 13 ++--
> tests/qtest/virtio-9p-test.c | 2 +-
> 15 files changed, 292 insertions(+), 26 deletions(-)
> create mode 100644 hw/9pfs/9p-util-darwin.c
> rename hw/9pfs/{9p-util.c => 9p-util-linux.c} (90%)
Queued on 9p.next:
https://github.com/cschoenebeck/qemu/commits/9p.next
Thanks!
Best regards,
Christian Schoenebeck
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v9 00/11] 9p: Add support for darwin
2022-03-01 19:25 ` [PATCH v9 00/11] 9p: Add support for darwin Christian Schoenebeck
@ 2022-03-01 20:09 ` Will Cohen
2022-03-02 18:15 ` Christian Schoenebeck
0 siblings, 1 reply; 27+ messages in thread
From: Will Cohen @ 2022-03-01 20:09 UTC (permalink / raw)
To: Christian Schoenebeck
Cc: Laurent Vivier, Thomas Huth, Philippe Mathieu-Daudé,
qemu Developers, hi, Paolo Bonzini, Greg Kurz
[-- Attachment #1: Type: text/plain, Size: 2791 bytes --]
On Tue, Mar 1, 2022 at 2:25 PM Christian Schoenebeck <qemu_oss@crudebyte.com>
wrote:
> On Sonntag, 27. Februar 2022 23:35:11 CET Will Cohen wrote:
> > This is a followup to
> > https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg04391.html,
> adding
> > 9p server support for Darwin.
> >
> > Since v8, the following changes have been made:
> >
> > Patch 4/11 (9p: darwin: Handle struct dirent differences)
> > - Declare qemu_dirent_off as static to prevent linker error
> > - Move qemu_dirent_off above the end-of-file endif to fix compilation
> >
> > Patch 9/11 (9p: darwin: Implement compatibility for mknodat)
> > - Fix line over 90 characters formatting error
> > - Move qemu_mknodat back from osdep to 9p-util and adjust patch notes
> > accordingly
> >
> > Patch 11/11 (9p: darwin: meson: Allow VirtFS on Darwin)
> > - Rebase to master
> >
> > With these changes, this patch set builds and passes 9p synth tests on
> both
> > linux and darwin.
> >
> > Keno Fischer (10):
> > 9p: linux: Fix a couple Linux assumptions
> > 9p: Rename 9p-util -> 9p-util-linux
> > 9p: darwin: Handle struct stat(fs) differences
> > 9p: darwin: Handle struct dirent differences
> > 9p: darwin: Ignore O_{NOATIME, DIRECT}
> > 9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX
> > 9p: darwin: *xattr_nofollow implementations
> > 9p: darwin: Compatibility for f/l*xattr
> > 9p: darwin: Implement compatibility for mknodat
> > 9p: darwin: meson: Allow VirtFS on Darwin
> >
> > Will Cohen (1):
> > 9p: darwin: Adjust assumption on virtio-9p-test
> >
> > fsdev/file-op-9p.h | 9 ++-
> > fsdev/meson.build | 1 +
> > hw/9pfs/9p-local.c | 27 +++++--
> > hw/9pfs/9p-proxy.c | 38 +++++++++-
> > hw/9pfs/9p-synth.c | 6 ++
> > hw/9pfs/9p-util-darwin.c | 97 ++++++++++++++++++++++++++
> > hw/9pfs/{9p-util.c => 9p-util-linux.c} | 8 ++-
> > hw/9pfs/9p-util.h | 46 ++++++++++++
> > hw/9pfs/9p.c | 42 +++++++++--
> > hw/9pfs/9p.h | 18 +++++
> > hw/9pfs/codir.c | 4 +-
> > hw/9pfs/meson.build | 3 +-
> > include/qemu/xattr.h | 4 +-
> > meson.build | 13 ++--
> > tests/qtest/virtio-9p-test.c | 2 +-
> > 15 files changed, 292 insertions(+), 26 deletions(-)
> > create mode 100644 hw/9pfs/9p-util-darwin.c
> > rename hw/9pfs/{9p-util.c => 9p-util-linux.c} (90%)
>
> Queued on 9p.next:
> https://github.com/cschoenebeck/qemu/commits/9p.next
>
> Thanks!
>
>
This is very exciting. Many, many thanks for helping guide this through the
process!
> Best regards,
> Christian Schoenebeck
>
>
>
[-- Attachment #2: Type: text/html, Size: 4017 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v9 00/11] 9p: Add support for darwin
2022-03-01 20:09 ` Will Cohen
@ 2022-03-02 18:15 ` Christian Schoenebeck
0 siblings, 0 replies; 27+ messages in thread
From: Christian Schoenebeck @ 2022-03-02 18:15 UTC (permalink / raw)
To: qemu-devel
Cc: Will Cohen, Laurent Vivier, Thomas Huth,
Philippe Mathieu-Daudé,
hi, Paolo Bonzini, Greg Kurz
On Dienstag, 1. März 2022 21:09:27 CET Will Cohen wrote:
> On Tue, Mar 1, 2022 at 2:25 PM Christian Schoenebeck
> <qemu_oss@crudebyte.com>
> wrote:
> > On Sonntag, 27. Februar 2022 23:35:11 CET Will Cohen wrote:
> > > This is a followup to
> > > https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg04391.html,
> >
> > adding
> >
> > > 9p server support for Darwin.
> > >
> > > Since v8, the following changes have been made:
> > >
> > > Patch 4/11 (9p: darwin: Handle struct dirent differences)
> > > - Declare qemu_dirent_off as static to prevent linker error
> > > - Move qemu_dirent_off above the end-of-file endif to fix compilation
> > >
> > > Patch 9/11 (9p: darwin: Implement compatibility for mknodat)
> > > - Fix line over 90 characters formatting error
> > > - Move qemu_mknodat back from osdep to 9p-util and adjust patch notes
> > > accordingly
> > >
> > > Patch 11/11 (9p: darwin: meson: Allow VirtFS on Darwin)
> > > - Rebase to master
> > >
> > > With these changes, this patch set builds and passes 9p synth tests on
> >
> > both
> >
> > > linux and darwin.
> > >
> > > Keno Fischer (10):
> > > 9p: linux: Fix a couple Linux assumptions
> > > 9p: Rename 9p-util -> 9p-util-linux
> > > 9p: darwin: Handle struct stat(fs) differences
> > > 9p: darwin: Handle struct dirent differences
> > > 9p: darwin: Ignore O_{NOATIME, DIRECT}
> > > 9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX
> > > 9p: darwin: *xattr_nofollow implementations
> > > 9p: darwin: Compatibility for f/l*xattr
> > > 9p: darwin: Implement compatibility for mknodat
> > > 9p: darwin: meson: Allow VirtFS on Darwin
> > >
> > > Will Cohen (1):
> > > 9p: darwin: Adjust assumption on virtio-9p-test
> > >
> > > fsdev/file-op-9p.h | 9 ++-
> > > fsdev/meson.build | 1 +
> > > hw/9pfs/9p-local.c | 27 +++++--
> > > hw/9pfs/9p-proxy.c | 38 +++++++++-
> > > hw/9pfs/9p-synth.c | 6 ++
> > > hw/9pfs/9p-util-darwin.c | 97 ++++++++++++++++++++++++++
> > > hw/9pfs/{9p-util.c => 9p-util-linux.c} | 8 ++-
> > > hw/9pfs/9p-util.h | 46 ++++++++++++
> > > hw/9pfs/9p.c | 42 +++++++++--
> > > hw/9pfs/9p.h | 18 +++++
> > > hw/9pfs/codir.c | 4 +-
> > > hw/9pfs/meson.build | 3 +-
> > > include/qemu/xattr.h | 4 +-
> > > meson.build | 13 ++--
> > > tests/qtest/virtio-9p-test.c | 2 +-
> > > 15 files changed, 292 insertions(+), 26 deletions(-)
> > > create mode 100644 hw/9pfs/9p-util-darwin.c
> > > rename hw/9pfs/{9p-util.c => 9p-util-linux.c} (90%)
> >
> > Queued on 9p.next:
> > https://github.com/cschoenebeck/qemu/commits/9p.next
> >
> > Thanks!
>
> This is very exciting. Many, many thanks for helping guide this through the
> process!
Thanks for not giving up. ;-)
I plan to send a PR on Friday to avoid the deadline traffic next week.
Best regards,
Christian Schoenebeck
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat
2022-02-27 22:35 ` [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat Will Cohen
2022-02-28 13:20 ` Christian Schoenebeck
@ 2022-04-08 13:52 ` Christian Schoenebeck
2022-04-08 15:00 ` Greg Kurz
1 sibling, 1 reply; 27+ messages in thread
From: Christian Schoenebeck @ 2022-04-08 13:52 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Greg Kurz,
Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Will Cohen, Paolo Bonzini, Keno Fischer,
Will Cohen
On Sonntag, 27. Februar 2022 23:35:20 CEST Will Cohen wrote:
> From: Keno Fischer <keno@juliacomputing.com>
>
> Darwin does not support mknodat. However, to avoid race conditions
> with later setting the permissions, we must avoid using mknod on
> the full path instead. We could try to fchdir, but that would cause
> problems if multiple threads try to call mknodat at the same time.
> However, luckily there is a solution: Darwin includes a function
> that sets the cwd for the current thread only.
> This should suffice to use mknod safely.
[...]
> diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> index cdb4c9e24c..bec0253474 100644
> --- a/hw/9pfs/9p-util-darwin.c
> +++ b/hw/9pfs/9p-util-darwin.c
> @@ -7,6 +7,8 @@
>
> #include "qemu/osdep.h"
> #include "qemu/xattr.h"
> +#include "qapi/error.h"
> +#include "qemu/error-report.h"
> #include "9p-util.h"
>
> ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char
> *name, @@ -62,3 +64,34 @@ int fsetxattrat_nofollow(int dirfd, const char
> *filename, const char *name, close_preserve_errno(fd);
> return ret;
> }
> +
> +/*
> + * As long as mknodat is not available on macOS, this workaround
> + * using pthread_fchdir_np is needed.
> + *
> + * Radar filed with Apple for implementing mknodat:
> + * rdar://FB9862426 (https://openradar.appspot.com/FB9862426)
> + */
> +#if defined CONFIG_PTHREAD_FCHDIR_NP
> +
> +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
> +{
> + int preserved_errno, err;
> + if (!pthread_fchdir_np) {
> + error_report_once("pthread_fchdir_np() not available on this
> version of macOS"); + return -ENOTSUP;
> + }
> + if (pthread_fchdir_np(dirfd) < 0) {
> + return -1;
> + }
> + err = mknod(filename, mode, dev);
I just tested this on macOS Monterey and realized mknod() seems to require
admin privileges on macOS to work. So if you run QEMU as ordinary user on
macOS then mknod() would fail with errno=1 (Operation not permitted).
That means a lot of stuff would simply not work on macOS, unless you really
want to run QEMU with super user privileges, which does not sound appealing to
me. :/
Should we introduce another fake behaviour here, i.e. remapping this on macOS
hosts as regular file and make guest believe it would create a device, similar
as we already do for mapped links?
> + preserved_errno = errno;
> + /* Stop using the thread-local cwd */
> + pthread_fchdir_np(-1);
> + if (err < 0) {
> + errno = preserved_errno;
> + }
> + return err;
> +}
> +
> +#endif
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat
2022-04-08 13:52 ` Christian Schoenebeck
@ 2022-04-08 15:00 ` Greg Kurz
2022-04-12 12:19 ` Christian Schoenebeck
0 siblings, 1 reply; 27+ messages in thread
From: Greg Kurz @ 2022-04-08 15:00 UTC (permalink / raw)
To: Christian Schoenebeck
Cc: Laurent Vivier, Thomas Huth, qemu-devel,
Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Will Cohen, Paolo Bonzini, Keno Fischer
On Fri, 08 Apr 2022 15:52:25 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> On Sonntag, 27. Februar 2022 23:35:20 CEST Will Cohen wrote:
> > From: Keno Fischer <keno@juliacomputing.com>
> >
> > Darwin does not support mknodat. However, to avoid race conditions
> > with later setting the permissions, we must avoid using mknod on
> > the full path instead. We could try to fchdir, but that would cause
> > problems if multiple threads try to call mknodat at the same time.
> > However, luckily there is a solution: Darwin includes a function
> > that sets the cwd for the current thread only.
> > This should suffice to use mknod safely.
> [...]
> > diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> > index cdb4c9e24c..bec0253474 100644
> > --- a/hw/9pfs/9p-util-darwin.c
> > +++ b/hw/9pfs/9p-util-darwin.c
> > @@ -7,6 +7,8 @@
> >
> > #include "qemu/osdep.h"
> > #include "qemu/xattr.h"
> > +#include "qapi/error.h"
> > +#include "qemu/error-report.h"
> > #include "9p-util.h"
> >
> > ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char
> > *name, @@ -62,3 +64,34 @@ int fsetxattrat_nofollow(int dirfd, const char
> > *filename, const char *name, close_preserve_errno(fd);
> > return ret;
> > }
> > +
> > +/*
> > + * As long as mknodat is not available on macOS, this workaround
> > + * using pthread_fchdir_np is needed.
> > + *
> > + * Radar filed with Apple for implementing mknodat:
> > + * rdar://FB9862426 (https://openradar.appspot.com/FB9862426)
> > + */
> > +#if defined CONFIG_PTHREAD_FCHDIR_NP
> > +
> > +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
> > +{
> > + int preserved_errno, err;
> > + if (!pthread_fchdir_np) {
> > + error_report_once("pthread_fchdir_np() not available on this
> > version of macOS"); + return -ENOTSUP;
> > + }
> > + if (pthread_fchdir_np(dirfd) < 0) {
> > + return -1;
> > + }
> > + err = mknod(filename, mode, dev);
>
> I just tested this on macOS Monterey and realized mknod() seems to require
> admin privileges on macOS to work. So if you run QEMU as ordinary user on
> macOS then mknod() would fail with errno=1 (Operation not permitted).
>
> That means a lot of stuff would simply not work on macOS, unless you really
> want to run QEMU with super user privileges, which does not sound appealing to
> me. :/
>
> Should we introduce another fake behaviour here, i.e. remapping this on macOS
> hosts as regular file and make guest believe it would create a device, similar
> as we already do for mapped links?
>
I'd rather keep that for the mapped security mode only to avoid
confusion, but qemu_mknodat() is also used in passthrough mode.
Anyway, it seems that macOS's mknod() only creates device files,
unlike linux (POSIX) which is also used to create FIFOs, sockets
and regular files. And it also requires elevated privileges,
CAP_MKNOD, in order to create device files.
It seems that this implementation of qemu_mknodat() is just missing
some features that can be implemented with unprivileged syscalls like
mkfifo(), socket() and open().
> > + preserved_errno = errno;
> > + /* Stop using the thread-local cwd */
> > + pthread_fchdir_np(-1);
> > + if (err < 0) {
> > + errno = preserved_errno;
> > + }
> > + return err;
> > +}
> > +
> > +#endif
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat
2022-04-08 15:00 ` Greg Kurz
@ 2022-04-12 12:19 ` Christian Schoenebeck
0 siblings, 0 replies; 27+ messages in thread
From: Christian Schoenebeck @ 2022-04-12 12:19 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, qemu-devel,
Philippe Mathieu-Daudé,
hi, Michael Roitzsch, Will Cohen, Paolo Bonzini, Keno Fischer,
Greg Kurz, Akihiko Odaki
On Freitag, 8. April 2022 17:00:59 CEST Greg Kurz wrote:
> On Fri, 08 Apr 2022 15:52:25 +0200
>
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > On Sonntag, 27. Februar 2022 23:35:20 CEST Will Cohen wrote:
> > > From: Keno Fischer <keno@juliacomputing.com>
> > >
> > > Darwin does not support mknodat. However, to avoid race conditions
> > > with later setting the permissions, we must avoid using mknod on
> > > the full path instead. We could try to fchdir, but that would cause
> > > problems if multiple threads try to call mknodat at the same time.
> > > However, luckily there is a solution: Darwin includes a function
> > > that sets the cwd for the current thread only.
> > > This should suffice to use mknod safely.
> >
> > [...]
> >
> > > diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> > > index cdb4c9e24c..bec0253474 100644
> > > --- a/hw/9pfs/9p-util-darwin.c
> > > +++ b/hw/9pfs/9p-util-darwin.c
> > > @@ -7,6 +7,8 @@
> > >
> > > #include "qemu/osdep.h"
> > > #include "qemu/xattr.h"
> > >
> > > +#include "qapi/error.h"
> > > +#include "qemu/error-report.h"
> > >
> > > #include "9p-util.h"
> > >
> > > ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const
> > > char
> > >
> > > *name, @@ -62,3 +64,34 @@ int fsetxattrat_nofollow(int dirfd, const char
> > > *filename, const char *name, close_preserve_errno(fd);
> > >
> > > return ret;
> > >
> > > }
> > >
> > > +
> > > +/*
> > > + * As long as mknodat is not available on macOS, this workaround
> > > + * using pthread_fchdir_np is needed.
> > > + *
> > > + * Radar filed with Apple for implementing mknodat:
> > > + * rdar://FB9862426 (https://openradar.appspot.com/FB9862426)
> > > + */
> > > +#if defined CONFIG_PTHREAD_FCHDIR_NP
> > > +
> > > +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t
> > > dev)
> > > +{
> > > + int preserved_errno, err;
> > > + if (!pthread_fchdir_np) {
> > > + error_report_once("pthread_fchdir_np() not available on this
> > > version of macOS"); + return -ENOTSUP;
> > > + }
> > > + if (pthread_fchdir_np(dirfd) < 0) {
> > > + return -1;
> > > + }
> > > + err = mknod(filename, mode, dev);
> >
> > I just tested this on macOS Monterey and realized mknod() seems to require
> > admin privileges on macOS to work. So if you run QEMU as ordinary user on
> > macOS then mknod() would fail with errno=1 (Operation not permitted).
> >
> > That means a lot of stuff would simply not work on macOS, unless you
> > really
> > want to run QEMU with super user privileges, which does not sound
> > appealing to me. :/
> >
> > Should we introduce another fake behaviour here, i.e. remapping this on
> > macOS hosts as regular file and make guest believe it would create a
> > device, similar as we already do for mapped links?
>
> I'd rather keep that for the mapped security mode only to avoid
> confusion, but qemu_mknodat() is also used in passthrough mode.
>
> Anyway, it seems that macOS's mknod() only creates device files,
> unlike linux (POSIX) which is also used to create FIFOs, sockets
> and regular files. And it also requires elevated privileges,
> CAP_MKNOD, in order to create device files.
>
> It seems that this implementation of qemu_mknodat() is just missing
> some features that can be implemented with unprivileged syscalls like
> mkfifo(), socket() and open().
+Akihiko on CC.
Like always when it comes to POSIX APIs, Apple's documentation on this is far
from being great. I actually had to test out what's supported with mknod() on
macOS, in which way, and what is not (tested on macOS 12 "Monterey" only):
* S_IFIFO: works, even as regular user.
* S_IFREG: doesn't work, neither as regular user (ERRNO 1, Operation not
permitted), nor as super-user (ERRNO 22, Invalid argument). So we should
divert that to a regular open() call on macOS.
* S_IFCHR and S_IFBLK: works as super-user, doesn't work for regular user
(ERRNO 1, Operation not permitted). So if 9p is used with passthrough
permissions, we should probably stick to the direct mknod() call and that
the user would need to run QEMU as super-user to get this working. Whereas
if 9p is used with mapped permissions, we should fake those devices by
creating regular files, store their type and major, minor numbers there and
that's it. We don't expect that guest ever tries to read/write such block/
character devices, right? I.e. I'm assuming that any read/write is handled
as an overlay by Linux kernel on its guest level, correct?
* S_IFSOCK: doesn't work, neither as regular user (ERRNO 1, Operation not
permitted), nor as super-user (ERRNO 22, Invalid argument). So we should
divert that to a socket() call on macOS.
Thoughts?
Best regards,
Christian Schoenebeck
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2022-04-12 12:23 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-27 22:35 [PATCH v9 00/11] 9p: Add support for darwin Will Cohen
2022-02-27 22:35 ` [PATCH v9 01/11] 9p: linux: Fix a couple Linux assumptions Will Cohen
2022-02-27 22:35 ` [PATCH v9 02/11] 9p: Rename 9p-util -> 9p-util-linux Will Cohen
2022-02-27 22:35 ` [PATCH v9 03/11] 9p: darwin: Handle struct stat(fs) differences Will Cohen
2022-02-27 22:35 ` [PATCH v9 04/11] 9p: darwin: Handle struct dirent differences Will Cohen
2022-02-27 22:35 ` [PATCH v9 05/11] 9p: darwin: Ignore O_{NOATIME, DIRECT} Will Cohen
2022-02-27 22:35 ` [PATCH v9 06/11] 9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX Will Cohen
2022-02-27 22:35 ` [PATCH v9 07/11] 9p: darwin: *xattr_nofollow implementations Will Cohen
2022-02-27 22:35 ` [PATCH v9 08/11] 9p: darwin: Compatibility for f/l*xattr Will Cohen
2022-02-27 22:35 ` [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat Will Cohen
2022-02-28 13:20 ` Christian Schoenebeck
2022-02-28 13:36 ` Thomas Huth
2022-02-28 13:51 ` Christian Schoenebeck
2022-02-28 14:06 ` Peter Maydell
2022-02-28 15:45 ` Christian Schoenebeck
2022-02-28 13:37 ` Will Cohen
2022-02-28 13:41 ` Greg Kurz
2022-04-08 13:52 ` Christian Schoenebeck
2022-04-08 15:00 ` Greg Kurz
2022-04-12 12:19 ` Christian Schoenebeck
2022-02-27 22:35 ` [PATCH v9 10/11] 9p: darwin: Adjust assumption on virtio-9p-test Will Cohen
2022-02-27 22:35 ` [PATCH v9 11/11] 9p: darwin: meson: Allow VirtFS on Darwin Will Cohen
2022-02-28 13:11 ` Christian Schoenebeck
2022-02-28 13:43 ` Will Cohen
2022-03-01 19:25 ` [PATCH v9 00/11] 9p: Add support for darwin Christian Schoenebeck
2022-03-01 20:09 ` Will Cohen
2022-03-02 18:15 ` Christian Schoenebeck
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.