[Qemu-devel] [PATCH] add futimesat syscall

[Qemu-devel] [PATCH] add futimesat syscall

[Riku Voipio]

[-- Attachment #1: Type: text/plain, Size: 166 bytes --]

Add futimes patch, originally from scratchbox qemu devkit.

Signed-off-by: Riku Voipio <riku.voipio@iki.fi>

-- 
"rm -rf" only sounds scary if you don't have backups

[-- Attachment #2: 23_futimesat.patch --]
[-- Type: text/plain, Size: 1795 bytes --]

Index: trunk/linux-user/syscall.c
===================================================================
--- trunk.orig/linux-user/syscall.c	2008-09-16 18:38:15.000000000 +0300
+++ trunk/linux-user/syscall.c	2008-09-17 20:07:40.000000000 +0300
@@ -156,6 +156,7 @@
 #define __NR_sys_faccessat __NR_faccessat
 #define __NR_sys_fchmodat __NR_fchmodat
 #define __NR_sys_fchownat __NR_fchownat
+#define __NR_sys_futimesat __NR_futimesat
 #define __NR_sys_getcwd1 __NR_getcwd
 #define __NR_sys_getdents __NR_getdents
 #define __NR_sys_getdents64 __NR_getdents64
@@ -200,6 +201,10 @@
 _syscall5(int,sys_fchownat,int,dirfd,const char *,pathname,
           uid_t,owner,gid_t,group,int,flags)
 #endif
+#if defined(TARGET_NR_futimesat) && defined(__NR_futimesat)
+_syscall3(int,sys_futimesat,int,dirfd,const char *,pathname,
+          const struct timeval *,times)
+#endif
 _syscall2(int,sys_getcwd1,char *,buf,size_t,size)
 #if TARGET_ABI_BITS == 32
 _syscall3(int, sys_getdents, uint, fd, struct dirent *, dirp, uint, count);
@@ -5743,6 +5748,25 @@
 	break;
 #endif
 
+#if defined(TARGET_NR_futimesat) && defined(__NR_futimesat)
+    case TARGET_NR_futimesat:
+        {
+            struct timeval tv[2];
+            if (copy_from_user_timeval(tv, arg3)
+                || copy_from_user_timeval(tv+1, arg3+sizeof(struct target_timeval))) {
+                goto efault;
+            }
+            if (!arg2)
+                ret = get_errno(sys_futimesat(arg1, NULL, tv));
+            else {
+                p = lock_user_string(arg2);
+                ret = get_errno(sys_futimesat(arg1, path(p), tv));
+                unlock_user(p, arg2, 0);
+            }
+        }
+        break;
+#endif
+
 #ifdef TARGET_NR_set_robust_list
     case TARGET_NR_set_robust_list:
 	goto unimplemented_nowarn;

Re: [Qemu-devel] [PATCH] add futimesat syscall

[andrzej zaborowski]

2008/9/17 Riku Voipio <riku.voipio@iki.fi>:
> Add futimes patch, originally from scratchbox qemu devkit.

Kirill Shutemov submitted an imlementation of futimensat() at
http://lists.gnu.org/archive/html/qemu-devel/2008-09/msg00439.html ,
it looks cleaner to me.

I would like to commit Kirill's series at some point if nobody finds
any issues with them.

Regards

Re: [Qemu-devel] [PATCH] add futimesat syscall

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]

On Thu, Sep 18, 2008 at 01:54:21AM +0200, andrzej zaborowski wrote:
> 2008/9/17 Riku Voipio <riku.voipio@iki.fi>:
> > Add futimes patch, originally from scratchbox qemu devkit.
> 
> Kirill Shutemov submitted an imlementation of futimensat() at
> http://lists.gnu.org/archive/html/qemu-devel/2008-09/msg00439.html ,
> it looks cleaner to me.
> 
> I would like to commit Kirill's series at some point if nobody finds
> any issues with them.

Please, don't commit binfmt_misc related patch. I'll fix it soon.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] [PATCH] add futimesat syscall

[Riku Voipio]

On Thu, Sep 18, 2008 at 01:54:21AM +0200, andrzej zaborowski wrote:
> 2008/9/17 Riku Voipio <riku.voipio@iki.fi>:
> > Add futimes patch, originally from scratchbox qemu devkit.

> Kirill Shutemov submitted an imlementation of futimensat() at
> http://lists.gnu.org/archive/html/qemu-devel/2008-09/msg00439.html ,
> it looks cleaner to me.

Kirill's patch looks good to me.

-- 
"rm -rf" only sounds scary if you don't have backups

Re: [Qemu-devel] [PATCH] add futimesat syscall

[Laurent Desnogues]

On Thu, Sep 18, 2008 at 8:30 AM, Riku Voipio <riku.voipio@iki.fi> wrote:
> On Thu, Sep 18, 2008 at 01:54:21AM +0200, andrzej zaborowski wrote:
>> 2008/9/17 Riku Voipio <riku.voipio@iki.fi>:
>> > Add futimes patch, originally from scratchbox qemu devkit.
>
>> Kirill Shutemov submitted an imlementation of futimensat() at
>> http://lists.gnu.org/archive/html/qemu-devel/2008-09/msg00439.html ,
>> it looks cleaner to me.
>
> Kirill's patch looks good to me.

There's a functional difference between Kirill's patch and yours:

Kirill +            ret = get_errno(futimesat(arg1, p, tvp));
you   +                ret = get_errno(sys_futimesat(arg1, path(p), tv));


Laurent

Re: [Qemu-devel] [PATCH] add futimesat syscall

[Riku Voipio]

On Thu, Sep 18, 2008 at 09:23:20AM +0200, Laurent Desnogues wrote:
> On Thu, Sep 18, 2008 at 8:30 AM, Riku Voipio <riku.voipio@iki.fi> wrote:
> > On Thu, Sep 18, 2008 at 01:54:21AM +0200, andrzej zaborowski wrote:

> >> http://lists.gnu.org/archive/html/qemu-devel/2008-09/msg00439.html ,
> >> it looks cleaner to me.

> > Kirill's patch looks good to me.

> There's a functional difference between Kirill's patch and yours:

> Kirill +            ret = get_errno(futimesat(arg1, p, tvp));
> you   +                ret = get_errno(sys_futimesat(arg1, path(p), tv));

The sbox patch defines sys_futimesat earlier in the file, presumably to
deal with supporting a stone-age version of libc/kernel headers.
path() is related to interp_prefix. The old utimes handlers don't use
path() either, and it's unlikely that any library-loading routine uses
futimes(). Thus, I believe Kirill's version is more correct.

That said, path() should probably renamed to something more obvious
(interp_path() ?), or path mangling could be made more generic (allowing
things like chroot() emulation).

-- 
"rm -rf" only sounds scary if you don't have backups

Re: [Qemu-devel] [PATCH] add futimesat syscall

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 1501 bytes --]

On Thu, Sep 18, 2008 at 12:08:21PM +0300, Riku Voipio wrote:
> On Thu, Sep 18, 2008 at 09:23:20AM +0200, Laurent Desnogues wrote:
> > On Thu, Sep 18, 2008 at 8:30 AM, Riku Voipio <riku.voipio@iki.fi> wrote:
> > > On Thu, Sep 18, 2008 at 01:54:21AM +0200, andrzej zaborowski wrote:
> 
> > >> http://lists.gnu.org/archive/html/qemu-devel/2008-09/msg00439.html ,
> > >> it looks cleaner to me.
> 
> > > Kirill's patch looks good to me.
> 
> > There's a functional difference between Kirill's patch and yours:
> 
> > Kirill +            ret = get_errno(futimesat(arg1, p, tvp));
> > you   +                ret = get_errno(sys_futimesat(arg1, path(p), tv));
> 
> The sbox patch defines sys_futimesat earlier in the file, presumably to
> deal with supporting a stone-age version of libc/kernel headers.

I don't think that it's great idea. If libc/kernel headers doesn't provide
the syscall we shouldn't implement them. The exception is syscalls without
libc's wrapper. Like gettid(2).

> path() is related to interp_prefix. The old utimes handlers don't use
> path() either, and it's unlikely that any library-loading routine uses
> futimes(). Thus, I believe Kirill's version is more correct.

I have written futimeat based on utimes(). If utimes() emulation work 
properly with interp_prefix != "/", futimesat() also will. But I have
never test it with interp_prefix != "/"

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] [PATCH] add futimesat syscall

[Riku Voipio]

On Thu, Sep 18, 2008 at 12:28:31PM +0300, Kirill A. Shutemov wrote:
> On Thu, Sep 18, 2008 at 12:08:21PM +0300, Riku Voipio wrote:
> I don't think that it's great idea. If libc/kernel headers doesn't provide
> the syscall we shouldn't implement them. The exception is syscalls without
> libc's wrapper. Like gettid(2).

Well, defining syscalls appears to be a common practice in qemu for the
*at family of syscalls (openat, linkat, ...), so it doesn't seem that
far off for futimesat().

-- 
"rm -rf" only sounds scary if you don't have backups

Re: [Qemu-devel] [PATCH] add futimesat syscall

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

On Thu, Sep 18, 2008 at 12:42:34PM +0300, Riku Voipio wrote:
> On Thu, Sep 18, 2008 at 12:28:31PM +0300, Kirill A. Shutemov wrote:
> > On Thu, Sep 18, 2008 at 12:08:21PM +0300, Riku Voipio wrote:
> > I don't think that it's great idea. If libc/kernel headers doesn't provide
> > the syscall we shouldn't implement them. The exception is syscalls without
> > libc's wrapper. Like gettid(2).
> 
> Well, defining syscalls appears to be a common practice in qemu for the
> *at family of syscalls (openat, linkat, ...), so it doesn't seem that
> far off for futimesat().

*at syscalls ware implemented in linux 2.6.16. glibc's wrappers to this
syscalls were implemented in glibc 2.4. Both of them relesed more than two
and half years ago.

I think we can drop this crap now.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] [PATCH] add futimesat syscall

[Thiemo Seufer]

Kirill A. Shutemov wrote:
> On Thu, Sep 18, 2008 at 12:42:34PM +0300, Riku Voipio wrote:
> > On Thu, Sep 18, 2008 at 12:28:31PM +0300, Kirill A. Shutemov wrote:
> > > On Thu, Sep 18, 2008 at 12:08:21PM +0300, Riku Voipio wrote:
> > > I don't think that it's great idea. If libc/kernel headers doesn't provide
> > > the syscall we shouldn't implement them. The exception is syscalls without
> > > libc's wrapper. Like gettid(2).
> > 
> > Well, defining syscalls appears to be a common practice in qemu for the
> > *at family of syscalls (openat, linkat, ...), so it doesn't seem that
> > far off for futimesat().
> 
> *at syscalls ware implemented in linux 2.6.16. glibc's wrappers to this
> syscalls were implemented in glibc 2.4. Both of them relesed more than two
> and half years ago.
> 
> I think we can drop this crap now.

Current Debian stable uses glibc 2.3.6.


Thiemo

Re: [Qemu-devel] [PATCH] add futimesat syscall

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 1125 bytes --]

On Thu, Sep 18, 2008 at 01:13:40PM +0200, Thiemo Seufer wrote:
> Kirill A. Shutemov wrote:
> > On Thu, Sep 18, 2008 at 12:42:34PM +0300, Riku Voipio wrote:
> > > On Thu, Sep 18, 2008 at 12:28:31PM +0300, Kirill A. Shutemov wrote:
> > > > On Thu, Sep 18, 2008 at 12:08:21PM +0300, Riku Voipio wrote:
> > > > I don't think that it's great idea. If libc/kernel headers doesn't provide
> > > > the syscall we shouldn't implement them. The exception is syscalls without
> > > > libc's wrapper. Like gettid(2).
> > > 
> > > Well, defining syscalls appears to be a common practice in qemu for the
> > > *at family of syscalls (openat, linkat, ...), so it doesn't seem that
> > > far off for futimesat().
> > 
> > *at syscalls ware implemented in linux 2.6.16. glibc's wrappers to this
> > syscalls were implemented in glibc 2.4. Both of them relesed more than two
> > and half years ago.
> > 
> > I think we can drop this crap now.
> 
> Current Debian stable uses glibc 2.3.6.

Hm... Ok, I'll fix my patches.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]