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

[Christian Schoenebeck]

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;