Re: [PATCH v3 6/6] linux-user: Add strace support for printing arguments of fallocate()

[Laurent Vivier]

Le 11/06/2020 à 17:51, Filip Bozuta a écrit :
> From: Filip Bozuta <Filip.Bozuta@syrmia.com>
> 
> This patch implements strace argument printing functionality for following 
> syscall:
> 
>     *fallocate - manipulate file space
> 
>         int fallocate(int fd, int mode, off_t offset, off_t len)
>         man page: https://www.man7.org/linux/man-pages/man2/fallocate.2.html
> 
> Implementation notes:
> 
>     This syscall's second argument "mode" is composed of predefined values
>     which represent flags that determine the type of operation that is
>     to be performed on the file space. For that reason, a printing
>     function "print_fallocate" was stated in file "strace.list". This printing
>     function uses an already existing function "print_flags()" to print flags 
> of
>     the "mode" argument. These flags are stated inside an array "falloc_flags"
>     that contains values of type "struct flags". These values are instantiated
>     using an existing macro "FLAG_GENERIC()". Most of these flags are defined
>     after kernel version 3.0 which is why they are enwrapped in an #ifdef
>     directive.
>     The syscall's third ant fourth argument are of type "off_t" which can
>     cause variations between 32/64-bit architectures. To handle this 
> variation,
>     function "target_offset64()" was copied from file "strace.c" and used in
>     "print_fallocate" to print "off_t" arguments for 32-bit architectures.
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  linux-user/qemu.h      | 16 ++++++++++++++++
>  linux-user/strace.c    | 40 ++++++++++++++++++++++++++++++++++++++++
>  linux-user/strace.list |  2 +-
>  linux-user/syscall.c   | 16 ----------------
>  4 files changed, 57 insertions(+), 17 deletions(-)
> 
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index 8f938b8105..be67391ba4 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -670,6 +670,22 @@ static inline int is_error(abi_long ret)
>      return (abi_ulong)ret >= (abi_ulong)(-4096);
>  }
>  
> +#if TARGET_ABI_BITS == 32
> +static inline uint64_t target_offset64(uint32_t word0, uint32_t word1)
> +{
> +#ifdef TARGET_WORDS_BIGENDIAN
> +    return ((uint64_t)word0 << 32) | word1;
> +#else
> +    return ((uint64_t)word1 << 32) | word0;
> +#endif
> +}
> +#else /* TARGET_ABI_BITS == 32 */
> +static inline uint64_t target_offset64(uint64_t word0, uint64_t word1)
> +{
> +    return word0;
> +}
> +#endif /* TARGET_ABI_BITS != 32 */
> +
>  /**
>   * preexit_cleanup: housekeeping before the guest exits
>   *
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 40c17f7abe..5f370256e3 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1144,6 +1144,26 @@ UNUSED static struct flags statx_mask[] = {
>      FLAG_END,
>  };
>  
> +UNUSED static struct flags falloc_flags[] = {
> +    FLAG_GENERIC(FALLOC_FL_KEEP_SIZE),
> +    FLAG_GENERIC(FALLOC_FL_PUNCH_HOLE),
> +#ifdef FALLOC_FL_NO_HIDE_STALE
> +    FLAG_GENERIC(FALLOC_FL_NO_HIDE_STALE),
> +#endif
> +#ifdef FALLOC_FL_COLLAPSE_RANGE
> +    FLAG_GENERIC(FALLOC_FL_COLLAPSE_RANGE),
> +#endif
> +#ifdef FALLOC_FL_ZERO_RANGE
> +    FLAG_GENERIC(FALLOC_FL_ZERO_RANGE),
> +#endif
> +#ifdef FALLOC_FL_INSERT_RANGE
> +    FLAG_GENERIC(FALLOC_FL_INSERT_RANGE),
> +#endif
> +#ifdef FALLOC_FL_UNSHARE_RANGE
> +    FLAG_GENERIC(FALLOC_FL_UNSHARE_RANGE),
> +#endif
> +};
> +
>  /*
>   * print_xxx utility functions.  These are used to print syscall
>   * parameters in certain format.  All of these have parameter
> @@ -1561,6 +1581,26 @@ print_faccessat(const struct syscallname *name,
>  }
>  #endif
>  
> +#ifdef TARGET_NR_fallocate
> +static void
> +print_fallocate(const struct syscallname *name,
> +    abi_long arg0, abi_long arg1, abi_long arg2,
> +    abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    print_syscall_prologue(name);
> +    print_raw_param("%d", arg0, 0);
> +    print_flags(falloc_flags, arg1, 0);
> +#if TARGET_ABI_BITS == 32
> +    print_raw_param("%ld", target_offset64(arg2, arg3), 0);
> +    print_raw_param("%ld", target_offset64(arg4, arg5), 1);
> +#else
> +    print_raw_param("%ld", arg2, 0);
> +    print_raw_param("%ld", arg3, 1);
> +#endif
>

You have removed the PRIu64 and TARGET_ABI_FMT_ld from the v2. So my
Reviewed-by is not relevant for the v3.

Thanks,
Laurent