[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v10 3/3] linux-user: Add support for statx() sys
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-devel] [PATCH v10 3/3] linux-user: Add support for statx() syscall |
Date: |
Thu, 13 Jun 2019 12:46:07 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 |
Le 07/06/2019 à 12:35, Aleksandar Markovic a écrit :
> From: Aleksandar Rikalo <address@hidden>
>
> Implement support for translation of system call statx().
>
> The implementation is based on "best effort" approach: if host is
> capable of executing statx(), host statx() is used. If not, the
> implementation includes invoking other (more mature) system calls
> (from the same 'stat' family) on the host side to achieve as close
> as possible functionality.
>
> Support for statx() in kernel and glibc was, however, introduced
> at different points of time (the difference is more than a year):
>
> - kernel: Linux 4.11 (30 April 2017)
> - glibc: glibc 2.28 (1 Aug 2018)
>
> In this patch, the availability of statx() support is established
> via __NR_statx (if it is defined, statx() is considered available).
> This coincedes with statx() introduction in kernel.
>
> However, the structure statx definition may not be available for hosts
> with glibc older than 2.28 (it is, by design, to be defined in one of
> glibc headers), even though the full statx() functionality may be
> supported in kernel, if the kernel is not older than 4.11. Hence,
> a structure "target_statx" is defined in this patch, to remove that
> dependency on glibc headers, and to use statx() functionality as soon
> as the host kernel is capable of supporting it. Such structure statx
> definition is used for both target and host structures statx (of
> course, this doesn't mean the endian arrangement is the same on
> target and host, and endian conversion is done in all necessary
> cases).
>
> Signed-off-by: Aleksandar Rikalo <address@hidden>
> Signed-off-by: Aleksandar Markovic <address@hidden>
> ---
> linux-user/syscall.c | 135
> +++++++++++++++++++++++++++++++++++++++++++++-
> linux-user/syscall_defs.h | 37 +++++++++++++
> 2 files changed, 171 insertions(+), 1 deletion(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 82c08b6..b78ed45 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
...
> @@ -10182,6 +10226,95 @@ static abi_long do_syscall1(void *cpu_env, int num,
> abi_long arg1,
> ret = host_to_target_stat64(cpu_env, arg3, &st);
> return ret;
> #endif
> +#if defined(TARGET_NR_statx)
> + case TARGET_NR_statx:
> + {
> + struct target_statx *target_stx;
> + int dirfd = arg1;
> + int flags = arg3;
> +
> + p = lock_user_string(arg2);
> + if (p == NULL) {
> + return -TARGET_EFAULT;
> + }
> +#if defined(__NR_statx)
> + {
> + /*
> + * It is assumed that struct statx is arhitecture independent
s/arhitecture/architecture/
> + */
> + struct target_statx host_stx;
> + int mask = arg4;
> +
> + ret = get_errno(syscall(__NR_statx, dirfd, p, flags, mask,
> + &host_stx));
You should define sys_statx() using _syscall5() macro and use it.
> + if (!is_error(ret)) {
> + if (host_to_target_statx(&host_stx, arg5) != 0) {
> + unlock_user(p, arg2, 0);
> + return -TARGET_EFAULT;
> + }
> + }
> +
> + if (ret != TARGET_ENOSYS) {
ret != -TARGET_ENOSYS
> + unlock_user(p, arg2, 0);
> + return ret;
> + }
> + }
> +#endif
> + if ((p == NULL) || (*((char *)p) == 0)) {
You already checked above p is not NULL and exited with -TARGET_EFAULT.
BTW, do we really need to emulate the syscall if it is not available?
I think the user-space application calling statx() should be ready to
receive ENOSYS and define some kinds of fallback (like you do below). So
it should not be done by QEMU.
> + /*
> + * By file descriptor
> + */
> + if (flags & AT_EMPTY_PATH) {
> + unlock_user(p, arg2, 0);
> + return -TARGET_ENOENT;
> + }
> + ret = get_errno(fstat(dirfd, &st));
> + } else if (*((char *)p) == '/') {
> + /*
> + * By absolute pathname
> + */
> + ret = get_errno(stat(path(p), &st));
> + } else {
> + if (dirfd == AT_FDCWD) {
> + /*
> + * By pathname relative to the current working directory
> + */
> + ret = get_errno(stat(path(p), &st));
Why do we divide the case in two parts, fstatat() should work here too.
> + } else {
> + /*
> + * By 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);
> +
> + if (!is_error(ret)) {
> + if (!lock_user_struct(VERIFY_WRITE, target_stx, arg5, 0)) {
> + return -TARGET_EFAULT;
> + }
> + memset(target_stx, 0, sizeof(*target_stx));
> + __put_user(major(st.st_dev), &target_stx->stx_dev_major);
> + __put_user(minor(st.st_dev), &target_stx->stx_dev_minor);
> + __put_user(st.st_ino, &target_stx->stx_ino);
> + __put_user(st.st_mode, &target_stx->stx_mode);
> + __put_user(st.st_uid, &target_stx->stx_uid);
> + __put_user(st.st_gid, &target_stx->stx_gid);
> + __put_user(st.st_nlink, &target_stx->stx_nlink);
> + __put_user(major(st.st_rdev), &target_stx->stx_rdev_major);
> + __put_user(minor(st.st_rdev), &target_stx->stx_rdev_minor);
> + __put_user(st.st_size, &target_stx->stx_size);
> + __put_user(st.st_blksize, &target_stx->stx_blksize);
> + __put_user(st.st_blocks, &target_stx->stx_blocks);
> + __put_user(st.st_atime, &target_stx->stx_atime.tv_sec);
> + __put_user(st.st_mtime, &target_stx->stx_mtime.tv_sec);
> + __put_user(st.st_ctime, &target_stx->stx_ctime.tv_sec);
> + unlock_user_struct(target_stx, arg5, 1);
> + }
> + }
> + return ret;
> +#endif
Thanks,
Laurent
Re: [Qemu-devel] [PATCH v10 0/3] linux-user: A set of miscellaneous patches, Aleksandar Markovic, 2019/06/11