[PATCH v4 1/6] 9pfs: fix qemu_mknodat(S_IFREG) on macOS

Christian Schoenebeck posted 6 patches 3 years, 2 months ago
There is a newer version of this series
[PATCH v4 1/6] 9pfs: fix qemu_mknodat(S_IFREG) on macOS
Posted by Christian Schoenebeck 3 years, 2 months ago
mknod() on macOS does not support creating regular files, so
divert to openat_file() if S_IFREG is passed with mode argument.

Furthermore, 'man 2 mknodat' on Linux says: "Zero file type is
equivalent to type S_IFREG".

Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Will Cohen <wwcohen@gmail.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/9pfs/9p-util-darwin.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
index bec0253474..e24d09763a 100644
--- a/hw/9pfs/9p-util-darwin.c
+++ b/hw/9pfs/9p-util-darwin.c
@@ -77,6 +77,15 @@ int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
 int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
 {
     int preserved_errno, err;
+
+    if (S_ISREG(mode) || !(mode & S_IFMT)) {
+        int fd = openat_file(dirfd, filename, O_CREAT, mode);
+        if (fd == -1) {
+            return fd;
+        }
+        close(fd);
+        return 0;
+    }
     if (!pthread_fchdir_np) {
         error_report_once("pthread_fchdir_np() not available on this version of macOS");
         return -ENOTSUP;
-- 
2.32.0 (Apple Git-132)
Re: [PATCH v4 1/6] 9pfs: fix qemu_mknodat(S_IFREG) on macOS
Posted by Greg Kurz 3 years, 2 months ago
On Wed, 27 Apr 2022 20:54:04 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> mknod() on macOS does not support creating regular files, so
> divert to openat_file() if S_IFREG is passed with mode argument.
> 
> Furthermore, 'man 2 mknodat' on Linux says: "Zero file type is
> equivalent to type S_IFREG".
> 

Thinking again I have mixed feelings about this... qemu_mknodat()
should certainly match POSIX semantics, even non-portable, as
described in [1] but I'm not sure it should mimic linux-specific
behaviors.

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/mknod.html

> Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> Reviewed-by: Will Cohen <wwcohen@gmail.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/9pfs/9p-util-darwin.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> index bec0253474..e24d09763a 100644
> --- a/hw/9pfs/9p-util-darwin.c
> +++ b/hw/9pfs/9p-util-darwin.c
> @@ -77,6 +77,15 @@ int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
>  int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
>  {
>      int preserved_errno, err;
> +
> +    if (S_ISREG(mode) || !(mode & S_IFMT)) {

... so maybe I'd just check S_ISREG() here. Not a request, just food
for thought : sticking to POSIX semantics might help to make the code
more portable across all the new host supports that are showing up
these days.

> +        int fd = openat_file(dirfd, filename, O_CREAT, mode);
> +        if (fd == -1) {
> +            return fd;
> +        }
> +        close(fd);
> +        return 0;
> +    }
>      if (!pthread_fchdir_np) {
>          error_report_once("pthread_fchdir_np() not available on this version of macOS");
>          return -ENOTSUP;
Re: [PATCH v4 1/6] 9pfs: fix qemu_mknodat(S_IFREG) on macOS
Posted by Christian Schoenebeck 3 years, 2 months ago
On Mittwoch, 27. April 2022 22:16:12 CEST Greg Kurz wrote:
> On Wed, 27 Apr 2022 20:54:04 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > mknod() on macOS does not support creating regular files, so
> > divert to openat_file() if S_IFREG is passed with mode argument.
> > 
> > Furthermore, 'man 2 mknodat' on Linux says: "Zero file type is
> > equivalent to type S_IFREG".
> 
> Thinking again I have mixed feelings about this... qemu_mknodat()
> should certainly match POSIX semantics, even non-portable, as
> described in [1] but I'm not sure it should mimic linux-specific
> behaviors.
> 
> [1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/mknod.html
> 
> > Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > Reviewed-by: Will Cohen <wwcohen@gmail.com>
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> > ---
> > 
> >  hw/9pfs/9p-util-darwin.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> > index bec0253474..e24d09763a 100644
> > --- a/hw/9pfs/9p-util-darwin.c
> > +++ b/hw/9pfs/9p-util-darwin.c
> > @@ -77,6 +77,15 @@ int fsetxattrat_nofollow(int dirfd, const char
> > *filename, const char *name,> 
> >  int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
> >  {
> >  
> >      int preserved_errno, err;
> > 
> > +
> > +    if (S_ISREG(mode) || !(mode & S_IFMT)) {
> 
> ... so maybe I'd just check S_ISREG() here. Not a request, just food
> for thought : sticking to POSIX semantics might help to make the code
> more portable across all the new host supports that are showing up
> these days.

Well, handling !(mode & S_IFMT) (i.e. "zero file type") is needed "somewhere" 
for 9p2000.L to behave as expected on macOS hosts. So you might argue this 
could rather be handled in 9p.c instead and there only if protocol is exactly 
9p2000.L.

OTOH I currently don't see an issue here providing that default behaviour.

Best regards,
Christian Schoenebeck

> > +        int fd = openat_file(dirfd, filename, O_CREAT, mode);
> > +        if (fd == -1) {
> > +            return fd;
> > +        }
> > +        close(fd);
> > +        return 0;
> > +    }
> > 
> >      if (!pthread_fchdir_np) {
> >      
> >          error_report_once("pthread_fchdir_np() not available on this
> >          version of macOS"); return -ENOTSUP;