Re: [Qemu-devel] [PATCH v8 81/87] linux-user: Add support for statx() syscall for all platforms

[Aleksandar Markovic]

> From: Timothy Baldwin <address@hidden>
> Sent: Monday, August 20, 2018 9:48 AM
> 
> Subject: Re: [PATCH v8 81/87] linux-user: Add support for statx() syscall for 
> all platforms
> 
> On 13/08/18 18:53, Aleksandar Markovic wrote:
> > From: Aleksandar Rikalo <address@hidden>
> >
> > Implement support for translation of system call statx(). The
> > implementation includes invoking other (more mature) syscalls
> > (from the same 'stat' family) on the host side. This way,
> > problems of availability of statx() on the host side are
> > avoided.
> >
> > Signed-off-by: Aleksandar Markovic <address@hidden>
> > Signed-off-by: Stefan Markovic <address@hidden>
> > ---
> >   linux-user/syscall.c      | 121 
> > +++++++++++++++++++++++++++++++++++++++++++++-
> >   linux-user/syscall_defs.h |  38 +++++++++++++++
> >   2 files changed, 158 insertions(+), 1 deletion(-)
> >
> 
> 
> > +            if ((p == NULL) || (*((char *)p) == 0)) {
> > +                /* By file descriptor */
> > +                ret = get_errno(fstat(dirfd, &st));
> > +                unlock_user(p, arg2, 0);
> > +            } else if (*((char *)p) == '/') {
> > +                /* Absolute pathname */
> > +                ret = get_errno(stat(path(p), &st));
> > +                unlock_user(p, arg2, 0);
> > +            } else {
> > +                if (dirfd == AT_FDCWD) {
> > +                    /* Pathname relative to the current working directory 
> > */
> > +                    ret = get_errno(stat(path(p), &st));
> > +                    unlock_user(p, arg2, 0);
> > +                } else {
> > +                    /*
> > +                     * Pathname relative to the directory referred to by 
> > the
> > +                     * file descriptor dirfd
> > +                     */
> > +                    ret = get_errno(fstatat(dirfd, path(p), &st, flags));
> > +                    unlock_user(p, arg2, 0);
> > +                }
> > +            }
> 
> This doesn't correctly handle the flags argument, it is ignored unless a
> relative path and directory file descriptor is provided.

Hi, Timothy.

Agreed. The patch generally needs certain improvements wrt handling original 
statx() arguments. I would certianly add handling the mask argument.

> As such an implementation of lstat that uses statx will be broken.
> 
> Since fstatat exists since 2.6.16 this can be reduced to a call to fstatat.

I am not sure what you meant here. I think QEMU lstat() implementation does not 
use statx(). This implementation of statx() uses hosts's statx(), and, as a 
fallback, host's fstat(), stat(), and fstatat().

I would like to add that I think this patch should be submitted separately, out 
of this series.

Yours,
Aleksandar