| Message ID | 20220227223522.91937-10-wwcohen@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | 9p: Add support for darwin | expand |
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'))
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
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')) > > >
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')) > > > > > >
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
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
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
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
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 > >
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
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'))