[Qemu-devel] [PATCH] linux-user: Implement renameat2 when defined

[Qemu-devel] [PATCH] linux-user: Implement renameat2 when defined

[Palmer Dabbelt]

From: Palmer Dabbelt <palmer@sifive.com>

The RISC-V Linux port was recently accept upstream and will be released
as part of 4.15.  While working on our glibc port I discovered that
qemu's user-mode emulation doesn't support renameat2, which has replaced
rename as part of the default system call list for new architectures.
Since a bunch of commonly used functionality boils down to rename (and
now renameat2), we ended up with many failures.

This patch adds support for renameat2.  As I'm not familiar with QEMU
development, I haven't really testing anything more than a simple
"./configure; make" on the upstream codebase, but I did test this
against our (not yet upstream) QEMU port where it appears to work for
me.  I've just cobbled it together by copying the existing renameat
implementation, but as there appears to be no glibc wrapper for
renameat2 on either of the systems I've tried this on I just emited the
system call directly.

    commit b0da6d44157aa6e652de7634343708251ba64146
    Author: James Hogan <james.hogan@imgtec.com>
    Date:   Fri Apr 29 22:29:26 2016 +0100

        asm-generic: Drop renameat syscall from default list

        The newer renameat2 syscall provides all the functionality provided by
        the renameat syscall and adds flags, so future architectures won't need
        to include renameat.

        Therefore drop the renameat syscall from the generic syscall list unless
        __ARCH_WANT_RENAMEAT is defined by the architecture's unistd.h prior to
        including asm-generic/unistd.h, and adjust all architectures using the
        generic syscall list to define it so that no in-tree architectures are
        affected.

Michael, Sagar, and Bastian are the RISC-V QEMU guys, I just use it.
I've CC'd them all here to serve as sort of a surprise introduction for
everyone -- they haven't seen the patch before, so blame me for the
stupid parts :).

CC: Michael Clark <mjc@sifive.com>
CC: Sagar Karandikar <sagark@eecs.berkeley.edu>
CC: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 linux-user/syscall.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 11c9116c4a1e..ece574f47aca 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8342,6 +8342,21 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         }
         break;
 #endif
+#if defined(TARGET_NR_renameat2) && defined(__NR_renameat2)
+    case TARGET_NR_renameat2:
+        {
+            void *p2;
+            p  = lock_user_string(arg2);
+            p2 = lock_user_string(arg4);
+            if (!p || !p2)
+                ret = -TARGET_EFAULT;
+            else
+                ret = get_errno(syscall(__NR_renameat2, arg1, p, arg3, p2, arg5));
+            unlock_user(p2, arg4, 0);
+            unlock_user(p, arg2, 0);
+        }
+        break;
+#endif
 #ifdef TARGET_NR_mkdir
     case TARGET_NR_mkdir:
         if (!(p = lock_user_string(arg1)))
-- 
2.13.6

Re: [Qemu-devel] [PATCH] linux-user: Implement renameat2 when defined

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20171220002941.14560-1-palmer@dabbelt.com
Subject: [Qemu-devel] [PATCH] linux-user: Implement renameat2 when defined

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
bcff5a4829 linux-user: Implement renameat2 when defined

=== OUTPUT BEGIN ===
Checking PATCH 1/1: linux-user: Implement renameat2 when defined...
WARNING: architecture specific defines should be avoided
#57: FILE: linux-user/syscall.c:8345:
+#if defined(TARGET_NR_renameat2) && defined(__NR_renameat2)

ERROR: braces {} are necessary for all arms of this statement
#63: FILE: linux-user/syscall.c:8351:
+            if (!p || !p2)
[...]
+            else
[...]

WARNING: line over 80 characters
#66: FILE: linux-user/syscall.c:8354:
+                ret = get_errno(syscall(__NR_renameat2, arg1, p, arg3, p2, arg5));

total: 1 errors, 2 warnings, 21 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PATCH] linux-user: Implement renameat2 when defined

[Bastian Koppelmann]

On 12/20/2017 01:29 AM, Palmer Dabbelt wrote:
> From: Palmer Dabbelt <palmer@sifive.com>
> 
> The RISC-V Linux port was recently accept upstream and will be released
> as part of 4.15.  While working on our glibc port I discovered that
> qemu's user-mode emulation doesn't support renameat2, which has replaced
> rename as part of the default system call list for new architectures.
> Since a bunch of commonly used functionality boils down to rename (and
> now renameat2), we ended up with many failures.
> 
> This patch adds support for renameat2.  As I'm not familiar with QEMU
> development, I haven't really testing anything more than a simple
> "./configure; make" on the upstream codebase, but I did test this
> against our (not yet upstream) QEMU port where it appears to work for
> me.  I've just cobbled it together by copying the existing renameat
> implementation, but as there appears to be no glibc wrapper for
> renameat2 on either of the systems I've tried this on I just emited the
> system call directly.
> 

CC'ed linux-user maintainer Riku Voipio.

Cheers,
Bastian

Re: [Qemu-devel] [PATCH] linux-user: Implement renameat2 when defined

[Peter Maydell]

On 20 December 2017 at 00:29, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> +#if defined(TARGET_NR_renameat2) && defined(__NR_renameat2)
> +    case TARGET_NR_renameat2:
> +        {
> +            void *p2;
> +            p  = lock_user_string(arg2);
> +            p2 = lock_user_string(arg4);
> +            if (!p || !p2)
> +                ret = -TARGET_EFAULT;
> +            else
> +                ret = get_errno(syscall(__NR_renameat2, arg1, p, arg3, p2, arg5));
> +            unlock_user(p2, arg4, 0);
> +            unlock_user(p, arg2, 0);
> +        }
> +        break;
> +#endif

Should we have code to handle using plain renameat for flags==0
calls? renameat2() only arrived in 3.15 kernels, so as it stands
this patch will work on a smaller set of hosts than it might.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] linux-user: Implement renameat2 when defined

[Palmer Dabbelt]

On Thu, 21 Dec 2017 06:01:25 PST (-0800), peter.maydell@linaro.org wrote:
> On 20 December 2017 at 00:29, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> +#if defined(TARGET_NR_renameat2) && defined(__NR_renameat2)
>> +    case TARGET_NR_renameat2:
>> +        {
>> +            void *p2;
>> +            p  = lock_user_string(arg2);
>> +            p2 = lock_user_string(arg4);
>> +            if (!p || !p2)
>> +                ret = -TARGET_EFAULT;
>> +            else
>> +                ret = get_errno(syscall(__NR_renameat2, arg1, p, arg3, p2, arg5));
>> +            unlock_user(p2, arg4, 0);
>> +            unlock_user(p, arg2, 0);
>> +        }
>> +        break;
>> +#endif
>
> Should we have code to handle using plain renameat for flags==0
> calls? renameat2() only arrived in 3.15 kernels, so as it stands
> this patch will work on a smaller set of hosts than it might.

That sound sane: there isn't even a glibc wrapper for renameat2, so I assume 
most calls will be with flags==0.  Do you want me to re-spin the patch, or can 
you take it from here?