Re: [PATCH v4 09/11] 9p: darwin: Implement compatibility for mknodat
On Mon, Feb 7, 2022 at 5:48 PM Christian Schoenebeck
wrote:
> On Montag, 7. Februar 2022 22:07:34 CET Will Cohen wrote:
> > On Mon, Feb 7, 2022 at 9:21 AM Christian Schoenebeck
> >
> > wrote:
> > > On Montag, 7. Februar 2022 11:57:25 CET Dr. David Alan Gilbert wrote:
> > > > * Greg Kurz (gr...@kaod.org) wrote:
> > > > > On Mon, 7 Feb 2022 11:30:18 +0100
> > > > >
> > > > > Philippe Mathieu-Daudé wrote:
> > > > > > On 7/2/22 09:47, Greg Kurz wrote:
> > > > > > > On Sun, 6 Feb 2022 20:10:23 -0500
> > > > > > >
> > > > > > > Will Cohen wrote:
> > > > > > >> This patch set currently places it in 9p-util only because 9p
> is
> > >
> > > the
> > >
> > > > > > >> only
> > > > > > >> place where this issue seems to have come up so far and we
> were
> > >
> > > wary
> > >
> > > > > > >> of
> > > > > > >> editing files too far afield, but I have no attachment to its
> > > > > > >> specific
> > > > > > >> location!
> > > > > > >
> > > > > > > Inline comments are preferred on qemu-devel. Please don't top
> post
> > >
> > > !
> > >
> > > > > > > This complicates the review a lot.
> > > > > > >
> > > > > > > This is indeed a good candidate for osdep. This being said,
> unless
> > > > > > > there's
> > > > > > > some other user in the QEMU code base, it is acceptable to
> leave
> > > > > > > it
> > > > > > > under
> > > > > > > 9pfs.
> > > > > >
> > > > > > virtiofsd could eventually use it.
> > > > >
> > > > > Indeed but virtiofsd is for linux hosts only AFAICT and I'm not
> aware
> > >
> > > of
> > >
> > > > > any work to support any other host OS.
> > > > >
> > > > > Cc'ing virtio-fs people for inputs on this topic.
> > > >
> > > > Indeeed, there's a lot of Linux specific code in the virtiofsd - I
> know
> > > > people are interested in other platforms, but I'm not sure that's the
> > > > right starting point.
> > > >
> > > > Dave
> > >
> > > Agreeing with Greg here: i.e. I would have placed this into osdep, but
> I
> > > would
> > > not insist on it either.
> > >
> > > Best regards,
> > > Christian Schoenebeck
> >
> > This makes sense. A revised version of this patch, moving qemu_mknodat
> from
> > 9p-util to osdep and os-posix, is attached below. I'd appreciate any
> > feedback from those looped in here, so that the context isn't lost before
> > resubmitting as a v5 patch, especially since this is starting to touch
> > files outside of 9p.
> >
> > From c9713c87163da7c96b5357d0d85ac318ae3d3051 Mon Sep 17 00:00:00 2001
> > From: Keno Fischer
> > Date: Sat, 16 Jun 2018 20:56:55 -0400
> > Subject: [PATCH] 9p: darwin: Implement compatibility for mknodat
> >
> > 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 tihs series.
> >
> > Signed-off-by: Keno Fischer
> > Signed-off-by: Michael Roitzsch
> > [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
> > - Move qemu_mknodat from 9p-util to osdep and os-posix]
> > Signed-off-by: Will Cohen
> > ---
> > hw/9pfs/9p-local.c | 4 ++--
> > include/qemu/osdep.h | 10 ++
> > os-posix.c | 34 ++
> > 3 files changed, 46 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/include/qemu/osdep.h b/include/qemu/osdep.h
> > index d1660d67fa..f3a8367ece 100644
> > --- a/include/qemu/osdep.h
> > +++ b/inclu
Re: [PATCH v4 09/11] 9p: darwin: Implement compatibility for mknodat
On Montag, 7. Februar 2022 22:07:34 CET Will Cohen wrote:
> On Mon, Feb 7, 2022 at 9:21 AM Christian Schoenebeck
>
> wrote:
> > On Montag, 7. Februar 2022 11:57:25 CET Dr. David Alan Gilbert wrote:
> > > * Greg Kurz (gr...@kaod.org) wrote:
> > > > On Mon, 7 Feb 2022 11:30:18 +0100
> > > >
> > > > Philippe Mathieu-Daudé wrote:
> > > > > On 7/2/22 09:47, Greg Kurz wrote:
> > > > > > On Sun, 6 Feb 2022 20:10:23 -0500
> > > > > >
> > > > > > Will Cohen wrote:
> > > > > >> This patch set currently places it in 9p-util only because 9p is
> >
> > the
> >
> > > > > >> only
> > > > > >> place where this issue seems to have come up so far and we were
> >
> > wary
> >
> > > > > >> of
> > > > > >> editing files too far afield, but I have no attachment to its
> > > > > >> specific
> > > > > >> location!
> > > > > >
> > > > > > Inline comments are preferred on qemu-devel. Please don't top post
> >
> > !
> >
> > > > > > This complicates the review a lot.
> > > > > >
> > > > > > This is indeed a good candidate for osdep. This being said, unless
> > > > > > there's
> > > > > > some other user in the QEMU code base, it is acceptable to leave
> > > > > > it
> > > > > > under
> > > > > > 9pfs.
> > > > >
> > > > > virtiofsd could eventually use it.
> > > >
> > > > Indeed but virtiofsd is for linux hosts only AFAICT and I'm not aware
> >
> > of
> >
> > > > any work to support any other host OS.
> > > >
> > > > Cc'ing virtio-fs people for inputs on this topic.
> > >
> > > Indeeed, there's a lot of Linux specific code in the virtiofsd - I know
> > > people are interested in other platforms, but I'm not sure that's the
> > > right starting point.
> > >
> > > Dave
> >
> > Agreeing with Greg here: i.e. I would have placed this into osdep, but I
> > would
> > not insist on it either.
> >
> > Best regards,
> > Christian Schoenebeck
>
> This makes sense. A revised version of this patch, moving qemu_mknodat from
> 9p-util to osdep and os-posix, is attached below. I'd appreciate any
> feedback from those looped in here, so that the context isn't lost before
> resubmitting as a v5 patch, especially since this is starting to touch
> files outside of 9p.
>
> From c9713c87163da7c96b5357d0d85ac318ae3d3051 Mon Sep 17 00:00:00 2001
> From: Keno Fischer
> Date: Sat, 16 Jun 2018 20:56:55 -0400
> Subject: [PATCH] 9p: darwin: Implement compatibility for mknodat
>
> 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 tihs series.
>
> Signed-off-by: Keno Fischer
> Signed-off-by: Michael Roitzsch
> [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
> - Move qemu_mknodat from 9p-util to osdep and os-posix]
> Signed-off-by: Will Cohen
> ---
> hw/9pfs/9p-local.c | 4 ++--
> include/qemu/osdep.h | 10 ++
> os-posix.c | 34 ++
> 3 files changed, 46 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/include/qemu/osdep.h b/include/qemu/osdep.h
> index d1660d67fa..f3a8367ece 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -810,3 +810,13 @@ static inline int
> platform_does_not_support_system(const char *command)
> #endif
>
> #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
> + */
> +#
Re: [PATCH v4 09/11] 9p: darwin: Implement compatibility for mknodat
On Mon, Feb 7, 2022 at 4:07 PM Will Cohen wrote:
> On Mon, Feb 7, 2022 at 9:21 AM Christian Schoenebeck <
> qemu_...@crudebyte.com> wrote:
>
>> On Montag, 7. Februar 2022 11:57:25 CET Dr. David Alan Gilbert wrote:
>> > * Greg Kurz (gr...@kaod.org) wrote:
>> > > On Mon, 7 Feb 2022 11:30:18 +0100
>> > >
>> > > Philippe Mathieu-Daudé wrote:
>> > > > On 7/2/22 09:47, Greg Kurz wrote:
>> > > > > On Sun, 6 Feb 2022 20:10:23 -0500
>> > > > >
>> > > > > Will Cohen wrote:
>> > > > >> This patch set currently places it in 9p-util only because 9p is
>> the
>> > > > >> only
>> > > > >> place where this issue seems to have come up so far and we were
>> wary
>> > > > >> of
>> > > > >> editing files too far afield, but I have no attachment to its
>> > > > >> specific
>> > > > >> location!
>> > > > >
>> > > > > Inline comments are preferred on qemu-devel. Please don't top
>> post !
>> > > > > This complicates the review a lot.
>> > > > >
>> > > > > This is indeed a good candidate for osdep. This being said, unless
>> > > > > there's
>> > > > > some other user in the QEMU code base, it is acceptable to leave
>> it
>> > > > > under
>> > > > > 9pfs.
>> > > >
>> > > > virtiofsd could eventually use it.
>> > >
>> > > Indeed but virtiofsd is for linux hosts only AFAICT and I'm not aware
>> of
>> > > any work to support any other host OS.
>> > >
>> > > Cc'ing virtio-fs people for inputs on this topic.
>> >
>> > Indeeed, there's a lot of Linux specific code in the virtiofsd - I know
>> > people are interested in other platforms, but I'm not sure that's the
>> > right starting point.
>> >
>> > Dave
>>
>> Agreeing with Greg here: i.e. I would have placed this into osdep, but I
>> would
>> not insist on it either.
>>
>> Best regards,
>> Christian Schoenebeck
>>
>>
> This makes sense. A revised version of this patch, moving qemu_mknodat
> from 9p-util to osdep and os-posix, is attached below. I'd appreciate any
> feedback from those looped in here, so that the context isn't lost before
> resubmitting as a v5 patch, especially since this is starting to touch
> files outside of 9p.
>
> From c9713c87163da7c96b5357d0d85ac318ae3d3051 Mon Sep 17 00:00:00 2001
> From: Keno Fischer
> Date: Sat, 16 Jun 2018 20:56:55 -0400
> Subject: [PATCH] 9p: darwin: Implement compatibility for mknodat
>
> 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 tihs series.
>
> Signed-off-by: Keno Fischer
> Signed-off-by: Michael Roitzsch
> [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
> - Move qemu_mknodat from 9p-util to osdep and os-posix]
> Signed-off-by: Will Cohen
> ---
> hw/9pfs/9p-local.c | 4 ++--
> include/qemu/osdep.h | 10 ++
> os-posix.c | 34 ++
> 3 files changed, 46 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/include/qemu/osdep.h b/include/qemu/osdep.h
> index d1660d67fa..f3a8367ece 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -810,3 +810,13 @@ static inline int
> platform_does_not_support_system(const char *command)
> #endif
>
> #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
> + */
> +#ifdef CONFIG_DARWIN
> +int pthread_fchdir_np(int fd);
> +#endif
> +int qemu_mknodat(int dirfd,
Re: [PATCH v4 09/11] 9p: darwin: Implement compatibility for mknodat
On Mon, Feb 7, 2022 at 9:21 AM Christian Schoenebeck
wrote:
> On Montag, 7. Februar 2022 11:57:25 CET Dr. David Alan Gilbert wrote:
> > * Greg Kurz (gr...@kaod.org) wrote:
> > > On Mon, 7 Feb 2022 11:30:18 +0100
> > >
> > > Philippe Mathieu-Daudé wrote:
> > > > On 7/2/22 09:47, Greg Kurz wrote:
> > > > > On Sun, 6 Feb 2022 20:10:23 -0500
> > > > >
> > > > > Will Cohen wrote:
> > > > >> This patch set currently places it in 9p-util only because 9p is
> the
> > > > >> only
> > > > >> place where this issue seems to have come up so far and we were
> wary
> > > > >> of
> > > > >> editing files too far afield, but I have no attachment to its
> > > > >> specific
> > > > >> location!
> > > > >
> > > > > Inline comments are preferred on qemu-devel. Please don't top post
> !
> > > > > This complicates the review a lot.
> > > > >
> > > > > This is indeed a good candidate for osdep. This being said, unless
> > > > > there's
> > > > > some other user in the QEMU code base, it is acceptable to leave it
> > > > > under
> > > > > 9pfs.
> > > >
> > > > virtiofsd could eventually use it.
> > >
> > > Indeed but virtiofsd is for linux hosts only AFAICT and I'm not aware
> of
> > > any work to support any other host OS.
> > >
> > > Cc'ing virtio-fs people for inputs on this topic.
> >
> > Indeeed, there's a lot of Linux specific code in the virtiofsd - I know
> > people are interested in other platforms, but I'm not sure that's the
> > right starting point.
> >
> > Dave
>
> Agreeing with Greg here: i.e. I would have placed this into osdep, but I
> would
> not insist on it either.
>
> Best regards,
> Christian Schoenebeck
>
>
This makes sense. A revised version of this patch, moving qemu_mknodat from
9p-util to osdep and os-posix, is attached below. I'd appreciate any
feedback from those looped in here, so that the context isn't lost before
resubmitting as a v5 patch, especially since this is starting to touch
files outside of 9p.
>From c9713c87163da7c96b5357d0d85ac318ae3d3051 Mon Sep 17 00:00:00 2001
From: Keno Fischer
Date: Sat, 16 Jun 2018 20:56:55 -0400
Subject: [PATCH] 9p: darwin: Implement compatibility for mknodat
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 tihs series.
Signed-off-by: Keno Fischer
Signed-off-by: Michael Roitzsch
[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
- Move qemu_mknodat from 9p-util to osdep and os-posix]
Signed-off-by: Will Cohen
---
hw/9pfs/9p-local.c | 4 ++--
include/qemu/osdep.h | 10 ++
os-posix.c | 34 ++
3 files changed, 46 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/include/qemu/osdep.h b/include/qemu/osdep.h
index d1660d67fa..f3a8367ece 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -810,3 +810,13 @@ static inline int
platform_does_not_support_system(const char *command)
#endif
#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
+ */
+#ifdef CONFIG_DARWIN
+int pthread_fchdir_np(int fd);
+#endif
+int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev);
diff --git a/os-posix.c b/os-posix.c
index ae6c9f2a5e..95c1607065 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -24,6 +24,7 @@
*/
#include "qemu/osdep.h"
+#include
#include
#include
#include
@@ -332,3 +333,36 @@ int os_mlock(void)
Re: [PATCH v4 09/11] 9p: darwin: Implement compatibility for mknodat
On Mon, Feb 07, 2022 at 11:49:12AM +0100, Greg Kurz wrote: > On Mon, 7 Feb 2022 11:30:18 +0100 > Philippe Mathieu-Daudé wrote: > > > On 7/2/22 09:47, Greg Kurz wrote: > > > On Sun, 6 Feb 2022 20:10:23 -0500 > > > Will Cohen wrote: > > > > > >> This patch set currently places it in 9p-util only because 9p is the only > > >> place where this issue seems to have come up so far and we were wary of > > >> editing files too far afield, but I have no attachment to its specific > > >> location! > > >> > > > > > > Inline comments are preferred on qemu-devel. Please don't top post ! > > > This complicates the review a lot. > > > > > > This is indeed a good candidate for osdep. This being said, unless there's > > > some other user in the QEMU code base, it is acceptable to leave it under > > > 9pfs. > > > > virtiofsd could eventually use it. > > > Indeed but virtiofsd is for linux hosts only AFAICT and I'm not aware of any > work to support any other host OS. [ CC Sergio ] Will like to support virtiofs on other host OS. Getting rid of Linux specific parts should be doable. I think bigger challenge is how to make vhost-user stuff work on other OS, like macOS. If virtiofsd was somehow running as part of qemu (and not as a separate process), then making rest of the filesystem code to work on other OS should not be too hard, I guess. So question is, can one somehow run same virtiofsd code both as part of qemu as well as separate daemon based on need (and one does not have to maintain two separate code bases). Thanks Vivek > > Cc'ing virtio-fs people for inputs on this topic. >
Re: [PATCH v4 09/11] 9p: darwin: Implement compatibility for mknodat
On Montag, 7. Februar 2022 11:57:25 CET Dr. David Alan Gilbert wrote: > * Greg Kurz (gr...@kaod.org) wrote: > > On Mon, 7 Feb 2022 11:30:18 +0100 > > > > Philippe Mathieu-Daudé wrote: > > > On 7/2/22 09:47, Greg Kurz wrote: > > > > On Sun, 6 Feb 2022 20:10:23 -0500 > > > > > > > > Will Cohen wrote: > > > >> This patch set currently places it in 9p-util only because 9p is the > > > >> only > > > >> place where this issue seems to have come up so far and we were wary > > > >> of > > > >> editing files too far afield, but I have no attachment to its > > > >> specific > > > >> location! > > > > > > > > Inline comments are preferred on qemu-devel. Please don't top post ! > > > > This complicates the review a lot. > > > > > > > > This is indeed a good candidate for osdep. This being said, unless > > > > there's > > > > some other user in the QEMU code base, it is acceptable to leave it > > > > under > > > > 9pfs. > > > > > > virtiofsd could eventually use it. > > > > Indeed but virtiofsd is for linux hosts only AFAICT and I'm not aware of > > any work to support any other host OS. > > > > Cc'ing virtio-fs people for inputs on this topic. > > Indeeed, there's a lot of Linux specific code in the virtiofsd - I know > people are interested in other platforms, but I'm not sure that's the > right starting point. > > Dave Agreeing with Greg here: i.e. I would have placed this into osdep, but I would not insist on it either. Best regards, Christian Schoenebeck
Re: [PATCH v4 09/11] 9p: darwin: Implement compatibility for mknodat
* Greg Kurz (gr...@kaod.org) wrote: > On Mon, 7 Feb 2022 11:30:18 +0100 > Philippe Mathieu-Daudé wrote: > > > On 7/2/22 09:47, Greg Kurz wrote: > > > On Sun, 6 Feb 2022 20:10:23 -0500 > > > Will Cohen wrote: > > > > > >> This patch set currently places it in 9p-util only because 9p is the only > > >> place where this issue seems to have come up so far and we were wary of > > >> editing files too far afield, but I have no attachment to its specific > > >> location! > > >> > > > > > > Inline comments are preferred on qemu-devel. Please don't top post ! > > > This complicates the review a lot. > > > > > > This is indeed a good candidate for osdep. This being said, unless there's > > > some other user in the QEMU code base, it is acceptable to leave it under > > > 9pfs. > > > > virtiofsd could eventually use it. > > > Indeed but virtiofsd is for linux hosts only AFAICT and I'm not aware of any > work to support any other host OS. > > Cc'ing virtio-fs people for inputs on this topic. Indeeed, there's a lot of Linux specific code in the virtiofsd - I know people are interested in other platforms, but I'm not sure that's the right starting point. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH v4 09/11] 9p: darwin: Implement compatibility for mknodat
On Mon, 7 Feb 2022 11:30:18 +0100 Philippe Mathieu-Daudé wrote: > On 7/2/22 09:47, Greg Kurz wrote: > > On Sun, 6 Feb 2022 20:10:23 -0500 > > Will Cohen wrote: > > > >> This patch set currently places it in 9p-util only because 9p is the only > >> place where this issue seems to have come up so far and we were wary of > >> editing files too far afield, but I have no attachment to its specific > >> location! > >> > > > > Inline comments are preferred on qemu-devel. Please don't top post ! > > This complicates the review a lot. > > > > This is indeed a good candidate for osdep. This being said, unless there's > > some other user in the QEMU code base, it is acceptable to leave it under > > 9pfs. > > virtiofsd could eventually use it. Indeed but virtiofsd is for linux hosts only AFAICT and I'm not aware of any work to support any other host OS. Cc'ing virtio-fs people for inputs on this topic.
Re: [PATCH v4 09/11] 9p: darwin: Implement compatibility for mknodat
On 7/2/22 09:47, Greg Kurz wrote: On Sun, 6 Feb 2022 20:10:23 -0500 Will Cohen wrote: This patch set currently places it in 9p-util only because 9p is the only place where this issue seems to have come up so far and we were wary of editing files too far afield, but I have no attachment to its specific location! Inline comments are preferred on qemu-devel. Please don't top post ! This complicates the review a lot. This is indeed a good candidate for osdep. This being said, unless there's some other user in the QEMU code base, it is acceptable to leave it under 9pfs. virtiofsd could eventually use it.
Re: [PATCH v4 09/11] 9p: darwin: Implement compatibility for mknodat
On Sun, 6 Feb 2022 20:10:23 -0500 Will Cohen wrote: > This patch set currently places it in 9p-util only because 9p is the only > place where this issue seems to have come up so far and we were wary of > editing files too far afield, but I have no attachment to its specific > location! > Inline comments are preferred on qemu-devel. Please don't top post ! This complicates the review a lot. This is indeed a good candidate for osdep. This being said, unless there's some other user in the QEMU code base, it is acceptable to leave it under 9pfs. > On Sun, Feb 6, 2022 at 4:21 PM Philippe Mathieu-Daudé > wrote: > > > On 6/2/22 21:07, Will Cohen wrote: > > > From: Keno Fischer > > > > > > 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 tihs series. > > > > > > Signed-off-by: Keno Fischer > > > Signed-off-by: Michael Roitzsch > > > [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] > > > Signed-off-by: Will Cohen > > > --- > > > hw/9pfs/9p-local.c | 5 +++-- > > > hw/9pfs/9p-util-darwin.c | 27 +++ > > > hw/9pfs/9p-util-linux.c | 5 + > > > hw/9pfs/9p-util.h| 2 ++ > > > 4 files changed, 37 insertions(+), 2 deletions(-) > > > > > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h > > > index 8e610ad224..f6fed963bf 100644 > > > --- a/hw/9pfs/9p-util.h > > > +++ b/hw/9pfs/9p-util.h > > > @@ -97,6 +97,8 @@ ssize_t flistxattrat_nofollow(int dirfd, const char > > *filename, > > > ssize_t fremovexattrat_nofollow(int dirfd, const char *filename, > > > const char *name); > > > > > > +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t > > dev); > > > > I think this belong to "osdep.h" & os-posix.c. > >
Re: [PATCH v4 09/11] 9p: darwin: Implement compatibility for mknodat
This patch set currently places it in 9p-util only because 9p is the only place where this issue seems to have come up so far and we were wary of editing files too far afield, but I have no attachment to its specific location! On Sun, Feb 6, 2022 at 4:21 PM Philippe Mathieu-Daudé wrote: > On 6/2/22 21:07, Will Cohen wrote: > > From: Keno Fischer > > > > 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 tihs series. > > > > Signed-off-by: Keno Fischer > > Signed-off-by: Michael Roitzsch > > [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] > > Signed-off-by: Will Cohen > > --- > > hw/9pfs/9p-local.c | 5 +++-- > > hw/9pfs/9p-util-darwin.c | 27 +++ > > hw/9pfs/9p-util-linux.c | 5 + > > hw/9pfs/9p-util.h| 2 ++ > > 4 files changed, 37 insertions(+), 2 deletions(-) > > > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h > > index 8e610ad224..f6fed963bf 100644 > > --- a/hw/9pfs/9p-util.h > > +++ b/hw/9pfs/9p-util.h > > @@ -97,6 +97,8 @@ ssize_t flistxattrat_nofollow(int dirfd, const char > *filename, > > ssize_t fremovexattrat_nofollow(int dirfd, const char *filename, > > const char *name); > > > > +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t > dev); > > I think this belong to "osdep.h" & os-posix.c. >
Re: [PATCH v4 09/11] 9p: darwin: Implement compatibility for mknodat
On 6/2/22 21:07, Will Cohen wrote: From: Keno Fischer 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 tihs series. Signed-off-by: Keno Fischer Signed-off-by: Michael Roitzsch [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] Signed-off-by: Will Cohen --- hw/9pfs/9p-local.c | 5 +++-- hw/9pfs/9p-util-darwin.c | 27 +++ hw/9pfs/9p-util-linux.c | 5 + hw/9pfs/9p-util.h| 2 ++ 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index 8e610ad224..f6fed963bf 100644 --- a/hw/9pfs/9p-util.h +++ b/hw/9pfs/9p-util.h @@ -97,6 +97,8 @@ ssize_t flistxattrat_nofollow(int dirfd, const char *filename, ssize_t fremovexattrat_nofollow(int dirfd, const char *filename, const char *name); +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev); I think this belong to "osdep.h" & os-posix.c.
