[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat
From: |
Christian Schoenebeck |
Subject: |
Re: [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat |
Date: |
Mon, 28 Feb 2022 14:51:58 +0100 |
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
- [PATCH v9 04/11] 9p: darwin: Handle struct dirent differences, (continued)
- [PATCH v9 04/11] 9p: darwin: Handle struct dirent differences, Will Cohen, 2022/02/27
- [PATCH v9 03/11] 9p: darwin: Handle struct stat(fs) differences, Will Cohen, 2022/02/27
- [PATCH v9 05/11] 9p: darwin: Ignore O_{NOATIME, DIRECT}, Will Cohen, 2022/02/27
- [PATCH v9 06/11] 9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX, Will Cohen, 2022/02/27
- [PATCH v9 07/11] 9p: darwin: *xattr_nofollow implementations, Will Cohen, 2022/02/27
- [PATCH v9 08/11] 9p: darwin: Compatibility for f/l*xattr, Will Cohen, 2022/02/27
- [PATCH v9 02/11] 9p: Rename 9p-util -> 9p-util-linux, Will Cohen, 2022/02/27
- [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat, Will Cohen, 2022/02/27
- Re: [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat, Will Cohen, 2022/02/28
- Re: [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat, Greg Kurz, 2022/02/28
[PATCH v9 10/11] 9p: darwin: Adjust assumption on virtio-9p-test, Will Cohen, 2022/02/27
[PATCH v9 11/11] 9p: darwin: meson: Allow VirtFS on Darwin, Will Cohen, 2022/02/27