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 <k...@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 <k...@juliacomputing.com>
> >> Signed-off-by: Michael Roitzsch <reactorcont...@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 <wwco...@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
