diff mbox

[1/3] linux-user: implement FS_IOC_GETFLAGS ioctl

Message ID 1345553012-19842-2-git-send-email-agraf@suse.de
State New
Headers show

Commit Message

Alexander Graf Aug. 21, 2012, 12:43 p.m. UTC
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 linux-user/ioctls.h       |    1 +
 linux-user/syscall_defs.h |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

Comments

Peter Maydell Aug. 21, 2012, 1:47 p.m. UTC | #1
On 21 August 2012 13:43, Alexander Graf <agraf@suse.de> wrote:
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  linux-user/ioctls.h       |    1 +
>  linux-user/syscall_defs.h |    2 ++
>  2 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> index bb76c56..1b798b3 100644
> --- a/linux-user/ioctls.h
> +++ b/linux-user/ioctls.h
> @@ -86,6 +86,7 @@
>       IOCTL_SPECIAL(FS_IOC_FIEMAP, IOC_W | IOC_R, do_ioctl_fs_ioc_fiemap,
>                     MK_PTR(MK_STRUCT(STRUCT_fiemap)))
>  #endif
> +  IOCTL(FS_IOC_GETFLAGS, IOC_R, MK_PTR(TYPE_INT))
>
>    IOCTL(SIOCATMARK, 0, TYPE_NULL)
>    IOCTL(SIOCADDRT, IOC_W, MK_PTR(MK_STRUCT(STRUCT_rtentry)))
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 3b7b1c3..67fbcab 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -2344,6 +2344,8 @@ struct target_eabi_flock64 {
>  #define TARGET_MTIOCGET        TARGET_IOR('m', 2, struct mtget)
>  #define TARGET_MTIOCPOS        TARGET_IOR('m', 3, struct mtpos)
>
> +#define TARGET_FS_IOC_GETFLAGS TARGET_IORU('f', 1)

This and the SETFLAGS one in the next patch fail the consistency
check that an x86_64-on-x86_64 linux-user binary performs:

cam-vm-266:precise:qemu$ ./x86_64-linux-user/qemu-x86_64 /bin/echo hello
ERROR: ioctl(FS_IOC_GETFLAGS): target=0x80046601 host=0x80086601
ERROR: ioctl(FS_IOC_SETFLAGS): target=0x40046602 host=0x40086602
hello

This is indicating that your ioctl definition is wrong:
> +  IOCTL(FS_IOC_GETFLAGS, IOC_R, MK_PTR(TYPE_INT))

...it should be TYPE_LONG.

Incidentally you could also set the target ioctl at compile
time rather than making syscall_init patch the size field
at runtime. I don't know whether one or the other is better
style... There's an argument for the 'automatic' one as
it enforces consistency and catches errors like the one above.
Anyway, the compile-time option woud be:

#define TARGET_FS_IOC_GETFLAGS TARGET_IOR('f', 1, abi_long)

(You could also squash patches 1 and 2 together IMHO.)

-- PMM
Riku Voipio Oct. 11, 2012, 1:36 p.m. UTC | #2
Hi Alexander,

On 21 August 2012 16:47, Peter Maydell <peter.maydell@linaro.org> wrote:
>> +#define TARGET_FS_IOC_GETFLAGS TARGET_IORU('f', 1)
>
> This and the SETFLAGS one in the next patch fail the consistency
> check that an x86_64-on-x86_64 linux-user binary performs:
>
> cam-vm-266:precise:qemu$ ./x86_64-linux-user/qemu-x86_64 /bin/echo hello
> ERROR: ioctl(FS_IOC_GETFLAGS): target=0x80046601 host=0x80086601
> ERROR: ioctl(FS_IOC_SETFLAGS): target=0x40046602 host=0x40086602
> hello
>
> This is indicating that your ioctl definition is wrong:
>> +  IOCTL(FS_IOC_GETFLAGS, IOC_R, MK_PTR(TYPE_INT))
>
> ...it should be TYPE_LONG.
>
> Incidentally you could also set the target ioctl at compile
> time rather than making syscall_init patch the size field
> at runtime. I don't know whether one or the other is better
> style... There's an argument for the 'automatic' one as
> it enforces consistency and catches errors like the one above.
> Anyway, the compile-time option woud be:
>
> #define TARGET_FS_IOC_GETFLAGS TARGET_IOR('f', 1, abi_long)
>
> (You could also squash patches 1 and 2 together IMHO.)

Has there been an updated patch with Peter's suggestions sent the list since?

Riku
Alexander Graf Oct. 11, 2012, 1:52 p.m. UTC | #3
On 11.10.2012, at 15:36, Riku Voipio wrote:

> Hi Alexander,
> 
> On 21 August 2012 16:47, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> +#define TARGET_FS_IOC_GETFLAGS TARGET_IORU('f', 1)
>> 
>> This and the SETFLAGS one in the next patch fail the consistency
>> check that an x86_64-on-x86_64 linux-user binary performs:
>> 
>> cam-vm-266:precise:qemu$ ./x86_64-linux-user/qemu-x86_64 /bin/echo hello
>> ERROR: ioctl(FS_IOC_GETFLAGS): target=0x80046601 host=0x80086601
>> ERROR: ioctl(FS_IOC_SETFLAGS): target=0x40046602 host=0x40086602
>> hello
>> 
>> This is indicating that your ioctl definition is wrong:
>>> +  IOCTL(FS_IOC_GETFLAGS, IOC_R, MK_PTR(TYPE_INT))
>> 
>> ...it should be TYPE_LONG.
>> 
>> Incidentally you could also set the target ioctl at compile
>> time rather than making syscall_init patch the size field
>> at runtime. I don't know whether one or the other is better
>> style... There's an argument for the 'automatic' one as
>> it enforces consistency and catches errors like the one above.
>> Anyway, the compile-time option woud be:
>> 
>> #define TARGET_FS_IOC_GETFLAGS TARGET_IOR('f', 1, abi_long)
>> 
>> (You could also squash patches 1 and 2 together IMHO.)
> 
> Has there been an updated patch with Peter's suggestions sent the list since?

Didn't get around to it yet, no.


Alex

> 
> Riku
diff mbox

Patch

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index bb76c56..1b798b3 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -86,6 +86,7 @@ 
      IOCTL_SPECIAL(FS_IOC_FIEMAP, IOC_W | IOC_R, do_ioctl_fs_ioc_fiemap,
                    MK_PTR(MK_STRUCT(STRUCT_fiemap)))
 #endif
+  IOCTL(FS_IOC_GETFLAGS, IOC_R, MK_PTR(TYPE_INT))
 
   IOCTL(SIOCATMARK, 0, TYPE_NULL)
   IOCTL(SIOCADDRT, IOC_W, MK_PTR(MK_STRUCT(STRUCT_rtentry)))
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 3b7b1c3..67fbcab 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2344,6 +2344,8 @@  struct target_eabi_flock64 {
 #define TARGET_MTIOCGET        TARGET_IOR('m', 2, struct mtget)
 #define TARGET_MTIOCPOS        TARGET_IOR('m', 3, struct mtpos)
 
+#define TARGET_FS_IOC_GETFLAGS TARGET_IORU('f', 1)
+
 struct target_sysinfo {
     abi_long uptime;                /* Seconds since boot */
     abi_ulong loads[3];             /* 1, 5, and 15 minute load averages */