Re: [Qemu-devel] [PATCH] linux-user: dup3, fallocate syscalls

[Riku Voipio <riku.voipio@iki.fi>]

Hi,

I quequed the other patches from you execpt this one -
The patch didn't apply for me, and a few comments inline:

On Thu, Jul 30, 2009 at 04:45:55PM +0200, Ulrich Hecht wrote:
> updated fallocate check to new configure, added dup3 check as suggested
> by Jan-Simon Möller

> Signed-off-by: Ulrich Hecht <uli@suse.de>
> ---
>  configure            |   36 ++++++++++++++++++++++++++++++++++++
>  linux-user/syscall.c |   12 ++++++++++++
>  2 files changed, 48 insertions(+), 0 deletions(-)
> 
> diff --git a/configure b/configure
> index 8160bed..2329f88 100755
> --- a/configure
> +++ b/configure
> @@ -1366,6 +1366,36 @@ if $cc $ARCH_CFLAGS -o $TMPE $TMPC 2> /dev/null ; then
>    splice=yes
>  fi
>  
> +# check for fallocate
> +fallocate=no
> +cat > $TMPC << EOF
> +#include <fcntl.h>
> +
> +int main(void)
> +{
> +    fallocate(0, 0, 0, 0);
> +    return 0;
> +}
> +EOF
> +if $cc $ARCH_CFLAGS -o $TMPE $TMPC 2> /dev/null ; then

There is new stanza for this in configure:

	if compile_prog "" "" ; then

> +  fallocate=yes
> +fi
> +
> +# check for dup3
> +dup3=no
> +cat > $TMPC << EOF
> +#include <unistd.h>
> +
> +int main(void)
> +{
> +    dup3(0, 0, 0);
> +    return 0;
> +}
> +EOF
> +if $cc $ARCH_CFLAGS -o $TMPE $TMPC 2> /dev/null ; then

likewise

> +  dup3=yes
> +fi
> +
>  # Check if tools are available to build documentation.
>  if test "$build_docs" = "yes" -a \( ! -x "`which texi2html 2>/dev/null`" -o ! -x "`which pod2man 2>/dev/null`" \) ; then
>    build_docs="no"
> @@ -1639,6 +1669,12 @@ fi
>  if test "$splice" = "yes" ; then
>    echo "CONFIG_SPLICE=y" >> $config_host_mak
>  fi
> +if test "$fallocate" = "yes" ; then
> +  echo "CONFIG_FALLOCATE=y" >> $config_host_mak
> +fi
> +if test "$dup3" = "yes" ; then
> +  echo "CONFIG_DUP3=y" >> $config_host_mak
> +fi
>  if test "$inotify" = "yes" ; then
>    echo "CONFIG_INOTIFY=y" >> $config_host_mak
>  fi
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 31cf151..d105b8e 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3681,8 +3681,10 @@ static int target_to_host_fcntl_cmd(int cmd)
>              return F_SETLEASE;
>          case TARGET_F_GETLEASE:
>              return F_GETLEASE;

This is patching your previous patch? Generally that is frowned upon - it
is tricky to find which patches need to be applied first, and it is better
to fix it in the original patch so the broken code is never part of the git
history.

> +#ifdef F_DUPFD_CLOEXEC
>          case TARGET_F_DUPFD_CLOEXEC:
>              return F_DUPFD_CLOEXEC;
> +#endif
>          case TARGET_F_NOTIFY:
>              return F_NOTIFY;
>  	default:
> @@ -4732,6 +4734,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>      case TARGET_NR_dup2:
>          ret = get_errno(dup2(arg1, arg2));
>          break;
> +#if defined(TARGET_NR_dup3) && defined(CONFIG_DUP3)
> +    case TARGET_NR_dup3:
> +        ret = get_errno(dup3(arg1, arg2, arg3));
> +        break;
> +#endif
>  #ifdef TARGET_NR_getppid /* not on alpha */
>      case TARGET_NR_getppid:
>          ret = get_errno(getppid());
> @@ -6976,6 +6983,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>          break;
>  #endif
>  #endif /* CONFIG_SPLICE */
> +#if defined(CONFIG_FALLOCATE) && defined(TARGET_NR_fallocate)
> +    case TARGET_NR_fallocate:
> +        ret = get_errno(fallocate(arg1, arg2, arg3, arg4));
> +        break;
> +#endif

This is colliding with other patches in linux-user-for-upstream. It is a good
example why it is better to separete patches for seperate features - In that case
it would have been quite easy for me to fix it in by hand.

>      default:
>      unimplemented:
>          gemu_log("qemu: Unsupported syscall: %d\n", num);

Cheers, Riku