Re: [Qemu-devel] [PATCH v2] linux-user: fix to handle variably sized SIOCGSTAMP with new kernels

2019-07-11 Thread Laurent Vivier 
Le 11/07/2019 à 11:24, Daniel P. Berrangé a écrit :
> On Thu, Jul 11, 2019 at 10:02:01AM +0200, Christian Ehrhardt wrote:
>> On Mon, Jun 17, 2019 at 5:35 PM Laurent Vivier  wrote:
>>>
>>> Le 17/06/2019 à 15:11, Daniel P. Berrangé a écrit :
 The SIOCGSTAMP symbol was previously defined in the
 asm-generic/sockios.h header file. QEMU sees that header
 indirectly via sys/socket.h

 In linux kernel commit 0768e17073dc527ccd18ed5f96ce85f9985e9115
 the asm-generic/sockios.h header no longer defines SIOCGSTAMP.
 Instead it provides only SIOCGSTAMP_OLD, which only uses a
 32-bit time_t on 32-bit architectures.

 The linux/sockios.h header then defines SIOCGSTAMP using
 either SIOCGSTAMP_OLD or SIOCGSTAMP_NEW as appropriate. If
 SIOCGSTAMP_NEW is used, then the tv_sec field is 64-bit even
 on 32-bit architectures

 To cope with this we must now define two separate syscalls,
 with corresponding old and new sizes, as well as including
 the new linux/sockios.h header.

 Signed-off-by: Daniel P. Berrangé 
 ---
  linux-user/ioctls.h| 15 +++
  linux-user/syscall.c   |  1 +
  linux-user/syscall_defs.h  |  5 +
  linux-user/syscall_types.h |  4 
  4 files changed, 25 insertions(+)

 diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
 index 5e84dc7c3a..5a6d6def7e 100644
 --- a/linux-user/ioctls.h
 +++ b/linux-user/ioctls.h
 @@ -222,8 +222,23 @@
IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
 +
 +#ifdef SIOCGSTAMP_OLD
 +  IOCTL(SIOCGSTAMP_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
 +#else
IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
 +#endif
 +#ifdef SIOCGSTAMPNS_OLD
 +  IOCTL(SIOCGSTAMPNS_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
 +#else
IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
 +#endif
 +#ifdef SIOCGSTAMP_NEW
 +  IOCTL(SIOCGSTAMP_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval64)))
 +#endif
 +#ifdef SIOCGSTAMPNS_NEW
 +  IOCTL(SIOCGSTAMPNS_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec64)))
 +#endif

IOCTL(RNDGETENTCNT, IOC_R, MK_PTR(TYPE_INT))
IOCTL(RNDADDTOENTCNT, IOC_W, MK_PTR(TYPE_INT))
 diff --git a/linux-user/syscall.c b/linux-user/syscall.c
 index b187c1281d..f13e260b02 100644
 --- a/linux-user/syscall.c
 +++ b/linux-user/syscall.c
 @@ -37,6 +37,7 @@
  #include 
  #include 
  #include 
 +#include 
  #include 
  #include 
  #include 
 diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
 index 7f141f699c..7830b600e7 100644
 --- a/linux-user/syscall_defs.h
 +++ b/linux-user/syscall_defs.h
 @@ -750,6 +750,11 @@ struct target_pollfd {

  #define TARGET_SIOCGSTAMP  0x8906  /* Get stamp (timeval) */
  #define TARGET_SIOCGSTAMPNS0x8907  /* Get stamp (timespec) */
 +#define TARGET_SIOCGSTAMP_OLD   0x8906  /* Get stamp (timeval) */
 +#define TARGET_SIOCGSTAMPNS_OLD 0x8907  /* Get stamp (timespec) */
 +#define TARGET_SIOCGSTAMP_NEW   TARGET_IOC(TARGET_IOC_READ, 's', 6, 
 sizeof(long long) + sizeof(long)) /* Get stamp (timeval64) */
 +#define TARGET_SIOCGSTAMPNS_NEW TARGET_IOC(TARGET_IOC_READ, 's', 7, 
 sizeof(long long) + sizeof(long)) /* Get stamp (timespec64) */
>>> kernel defines:
>>>
>>> #define SIOCGSTAMP_NEW   _IOR(SOCK_IOC_TYPE, 0x06, long long[2])
>>> #define SIOCGSTAMPNS_NEW _IOR(SOCK_IOC_TYPE, 0x07, long long[2])
>>>
>>> So it should be TARGET_IOR(0x89, 0x6, abi_llong[2])
>>>
>>> Their codes are 0x80108906 and 80108907.
>>
>> Hi,
>> I found the discussion around this topic being almost a month old.
>> And related to this fedora bug [1] was closed by adding [2] which
>> matches [3] that was nacked in the discussion here.
>>
>> Since I found nothing later (neither qemu commits nor further
>> discussions) I wonder if it has fallen through the cracks OR if there
>> was a kernel fix/change to resolve it (if that is the case a pointer
>> to the related kernel change would be nice)?
> 
> I didn't have time to address the feedback to this v2, and since the
> immediate problem for Fedora has a workaround, its been lower priority
> especially since my understanding of linux-iser is limited.
> 
> IOW, If anyone wants to take over my patch proposal here feel free. It
> would obviously be nice to fix for this 4.1 release if practical.

I'm going to try to update your patch.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v2] linux-user: fix to handle variably sized SIOCGSTAMP with new kernels

2019-07-11 Thread Christian Borntraeger 



On 11.07.19 11:24, Daniel P. Berrangé wrote:
> On Thu, Jul 11, 2019 at 10:02:01AM +0200, Christian Ehrhardt wrote:
>> On Mon, Jun 17, 2019 at 5:35 PM Laurent Vivier  wrote:
>>>
>>> Le 17/06/2019 à 15:11, Daniel P. Berrangé a écrit :
 The SIOCGSTAMP symbol was previously defined in the
 asm-generic/sockios.h header file. QEMU sees that header
 indirectly via sys/socket.h

 In linux kernel commit 0768e17073dc527ccd18ed5f96ce85f9985e9115
 the asm-generic/sockios.h header no longer defines SIOCGSTAMP.
 Instead it provides only SIOCGSTAMP_OLD, which only uses a
 32-bit time_t on 32-bit architectures.

 The linux/sockios.h header then defines SIOCGSTAMP using
 either SIOCGSTAMP_OLD or SIOCGSTAMP_NEW as appropriate. If
 SIOCGSTAMP_NEW is used, then the tv_sec field is 64-bit even
 on 32-bit architectures

 To cope with this we must now define two separate syscalls,
 with corresponding old and new sizes, as well as including
 the new linux/sockios.h header.

 Signed-off-by: Daniel P. Berrangé 
 ---
  linux-user/ioctls.h| 15 +++
  linux-user/syscall.c   |  1 +
  linux-user/syscall_defs.h  |  5 +
  linux-user/syscall_types.h |  4 
  4 files changed, 25 insertions(+)

 diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
 index 5e84dc7c3a..5a6d6def7e 100644
 --- a/linux-user/ioctls.h
 +++ b/linux-user/ioctls.h
 @@ -222,8 +222,23 @@
IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
 +
 +#ifdef SIOCGSTAMP_OLD
 +  IOCTL(SIOCGSTAMP_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
 +#else
IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
 +#endif
 +#ifdef SIOCGSTAMPNS_OLD
 +  IOCTL(SIOCGSTAMPNS_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
 +#else
IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
 +#endif
 +#ifdef SIOCGSTAMP_NEW
 +  IOCTL(SIOCGSTAMP_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval64)))
 +#endif
 +#ifdef SIOCGSTAMPNS_NEW
 +  IOCTL(SIOCGSTAMPNS_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec64)))
 +#endif

IOCTL(RNDGETENTCNT, IOC_R, MK_PTR(TYPE_INT))
IOCTL(RNDADDTOENTCNT, IOC_W, MK_PTR(TYPE_INT))
 diff --git a/linux-user/syscall.c b/linux-user/syscall.c
 index b187c1281d..f13e260b02 100644
 --- a/linux-user/syscall.c
 +++ b/linux-user/syscall.c
 @@ -37,6 +37,7 @@
  #include 
  #include 
  #include 
 +#include 
  #include 
  #include 
  #include 
 diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
 index 7f141f699c..7830b600e7 100644
 --- a/linux-user/syscall_defs.h
 +++ b/linux-user/syscall_defs.h
 @@ -750,6 +750,11 @@ struct target_pollfd {

  #define TARGET_SIOCGSTAMP  0x8906  /* Get stamp (timeval) */
  #define TARGET_SIOCGSTAMPNS0x8907  /* Get stamp (timespec) */
 +#define TARGET_SIOCGSTAMP_OLD   0x8906  /* Get stamp (timeval) */
 +#define TARGET_SIOCGSTAMPNS_OLD 0x8907  /* Get stamp (timespec) */
 +#define TARGET_SIOCGSTAMP_NEW   TARGET_IOC(TARGET_IOC_READ, 's', 6, 
 sizeof(long long) + sizeof(long)) /* Get stamp (timeval64) */
 +#define TARGET_SIOCGSTAMPNS_NEW TARGET_IOC(TARGET_IOC_READ, 's', 7, 
 sizeof(long long) + sizeof(long)) /* Get stamp (timespec64) */
>>> kernel defines:
>>>
>>> #define SIOCGSTAMP_NEW   _IOR(SOCK_IOC_TYPE, 0x06, long long[2])
>>> #define SIOCGSTAMPNS_NEW _IOR(SOCK_IOC_TYPE, 0x07, long long[2])
>>>
>>> So it should be TARGET_IOR(0x89, 0x6, abi_llong[2])
>>>
>>> Their codes are 0x80108906 and 80108907.
>>
>> Hi,
>> I found the discussion around this topic being almost a month old.
>> And related to this fedora bug [1] was closed by adding [2] which
>> matches [3] that was nacked in the discussion here.
>>
>> Since I found nothing later (neither qemu commits nor further
>> discussions) I wonder if it has fallen through the cracks OR if there
>> was a kernel fix/change to resolve it (if that is the case a pointer
>> to the related kernel change would be nice)?
> 
> I didn't have time to address the feedback to this v2, and since the
> immediate problem for Fedora has a workaround, its been lower priority
> especially since my understanding of linux-iser is limited.
> 
> IOW, If anyone wants to take over my patch proposal here feel free. It
> would obviously be nice to fix for this 4.1 release if practical.

Same for me. I have never dealt with linux-user and my workaround was the
simplest thing that I could come up with. Would be good it Laurent or other
linux-user experts could do the "right thing".


Adding Peter, for your awareness. qemu does not build with newer kernel headers.

> 
>>
>> [1]:

Re: [Qemu-devel] [PATCH v2] linux-user: fix to handle variably sized SIOCGSTAMP with new kernels

2019-07-11 Thread Daniel P . Berrangé 
On Thu, Jul 11, 2019 at 10:02:01AM +0200, Christian Ehrhardt wrote:
> On Mon, Jun 17, 2019 at 5:35 PM Laurent Vivier  wrote:
> >
> > Le 17/06/2019 à 15:11, Daniel P. Berrangé a écrit :
> > > The SIOCGSTAMP symbol was previously defined in the
> > > asm-generic/sockios.h header file. QEMU sees that header
> > > indirectly via sys/socket.h
> > >
> > > In linux kernel commit 0768e17073dc527ccd18ed5f96ce85f9985e9115
> > > the asm-generic/sockios.h header no longer defines SIOCGSTAMP.
> > > Instead it provides only SIOCGSTAMP_OLD, which only uses a
> > > 32-bit time_t on 32-bit architectures.
> > >
> > > The linux/sockios.h header then defines SIOCGSTAMP using
> > > either SIOCGSTAMP_OLD or SIOCGSTAMP_NEW as appropriate. If
> > > SIOCGSTAMP_NEW is used, then the tv_sec field is 64-bit even
> > > on 32-bit architectures
> > >
> > > To cope with this we must now define two separate syscalls,
> > > with corresponding old and new sizes, as well as including
> > > the new linux/sockios.h header.
> > >
> > > Signed-off-by: Daniel P. Berrangé 
> > > ---
> > >  linux-user/ioctls.h| 15 +++
> > >  linux-user/syscall.c   |  1 +
> > >  linux-user/syscall_defs.h  |  5 +
> > >  linux-user/syscall_types.h |  4 
> > >  4 files changed, 25 insertions(+)
> > >
> > > diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> > > index 5e84dc7c3a..5a6d6def7e 100644
> > > --- a/linux-user/ioctls.h
> > > +++ b/linux-user/ioctls.h
> > > @@ -222,8 +222,23 @@
> > >IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
> > >IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
> > >IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
> > > +
> > > +#ifdef SIOCGSTAMP_OLD
> > > +  IOCTL(SIOCGSTAMP_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
> > > +#else
> > >IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
> > > +#endif
> > > +#ifdef SIOCGSTAMPNS_OLD
> > > +  IOCTL(SIOCGSTAMPNS_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
> > > +#else
> > >IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
> > > +#endif
> > > +#ifdef SIOCGSTAMP_NEW
> > > +  IOCTL(SIOCGSTAMP_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval64)))
> > > +#endif
> > > +#ifdef SIOCGSTAMPNS_NEW
> > > +  IOCTL(SIOCGSTAMPNS_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec64)))
> > > +#endif
> > >
> > >IOCTL(RNDGETENTCNT, IOC_R, MK_PTR(TYPE_INT))
> > >IOCTL(RNDADDTOENTCNT, IOC_W, MK_PTR(TYPE_INT))
> > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > > index b187c1281d..f13e260b02 100644
> > > --- a/linux-user/syscall.c
> > > +++ b/linux-user/syscall.c
> > > @@ -37,6 +37,7 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >  #include 
> > >  #include 
> > >  #include 
> > > diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> > > index 7f141f699c..7830b600e7 100644
> > > --- a/linux-user/syscall_defs.h
> > > +++ b/linux-user/syscall_defs.h
> > > @@ -750,6 +750,11 @@ struct target_pollfd {
> > >
> > >  #define TARGET_SIOCGSTAMP  0x8906  /* Get stamp (timeval) */
> > >  #define TARGET_SIOCGSTAMPNS0x8907  /* Get stamp (timespec) */
> > > +#define TARGET_SIOCGSTAMP_OLD   0x8906  /* Get stamp (timeval) */
> > > +#define TARGET_SIOCGSTAMPNS_OLD 0x8907  /* Get stamp (timespec) 
> > > */
> > > +#define TARGET_SIOCGSTAMP_NEW   TARGET_IOC(TARGET_IOC_READ, 's', 6, 
> > > sizeof(long long) + sizeof(long)) /* Get stamp (timeval64) */
> > > +#define TARGET_SIOCGSTAMPNS_NEW TARGET_IOC(TARGET_IOC_READ, 's', 7, 
> > > sizeof(long long) + sizeof(long)) /* Get stamp (timespec64) */
> > kernel defines:
> >
> > #define SIOCGSTAMP_NEW   _IOR(SOCK_IOC_TYPE, 0x06, long long[2])
> > #define SIOCGSTAMPNS_NEW _IOR(SOCK_IOC_TYPE, 0x07, long long[2])
> >
> > So it should be TARGET_IOR(0x89, 0x6, abi_llong[2])
> >
> > Their codes are 0x80108906 and 80108907.
> 
> Hi,
> I found the discussion around this topic being almost a month old.
> And related to this fedora bug [1] was closed by adding [2] which
> matches [3] that was nacked in the discussion here.
> 
> Since I found nothing later (neither qemu commits nor further
> discussions) I wonder if it has fallen through the cracks OR if there
> was a kernel fix/change to resolve it (if that is the case a pointer
> to the related kernel change would be nice)?

I didn't have time to address the feedback to this v2, and since the
immediate problem for Fedora has a workaround, its been lower priority
especially since my understanding of linux-iser is limited.

IOW, If anyone wants to take over my patch proposal here feel free. It
would obviously be nice to fix for this 4.1 release if practical.

> 
> [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1718926
> [2]: 
> https://src.fedoraproject.org/rpms/qemu/blob/master/f/0005-NOT-UPSTREAM-Build-fix-with-latest-kernel.patch
> [3]: https://patchew.org/QEMU/20190604071915.288045-1-borntrae...@de.ibm.com/
> 
> > Thanks,
> > Laurent
> >

Re: [Qemu-devel] [PATCH v2] linux-user: fix to handle variably sized SIOCGSTAMP with new kernels

2019-07-11 Thread Christian Ehrhardt 
On Mon, Jun 17, 2019 at 5:35 PM Laurent Vivier  wrote:
>
> Le 17/06/2019 à 15:11, Daniel P. Berrangé a écrit :
> > The SIOCGSTAMP symbol was previously defined in the
> > asm-generic/sockios.h header file. QEMU sees that header
> > indirectly via sys/socket.h
> >
> > In linux kernel commit 0768e17073dc527ccd18ed5f96ce85f9985e9115
> > the asm-generic/sockios.h header no longer defines SIOCGSTAMP.
> > Instead it provides only SIOCGSTAMP_OLD, which only uses a
> > 32-bit time_t on 32-bit architectures.
> >
> > The linux/sockios.h header then defines SIOCGSTAMP using
> > either SIOCGSTAMP_OLD or SIOCGSTAMP_NEW as appropriate. If
> > SIOCGSTAMP_NEW is used, then the tv_sec field is 64-bit even
> > on 32-bit architectures
> >
> > To cope with this we must now define two separate syscalls,
> > with corresponding old and new sizes, as well as including
> > the new linux/sockios.h header.
> >
> > Signed-off-by: Daniel P. Berrangé 
> > ---
> >  linux-user/ioctls.h| 15 +++
> >  linux-user/syscall.c   |  1 +
> >  linux-user/syscall_defs.h  |  5 +
> >  linux-user/syscall_types.h |  4 
> >  4 files changed, 25 insertions(+)
> >
> > diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> > index 5e84dc7c3a..5a6d6def7e 100644
> > --- a/linux-user/ioctls.h
> > +++ b/linux-user/ioctls.h
> > @@ -222,8 +222,23 @@
> >IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
> >IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
> >IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
> > +
> > +#ifdef SIOCGSTAMP_OLD
> > +  IOCTL(SIOCGSTAMP_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
> > +#else
> >IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
> > +#endif
> > +#ifdef SIOCGSTAMPNS_OLD
> > +  IOCTL(SIOCGSTAMPNS_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
> > +#else
> >IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
> > +#endif
> > +#ifdef SIOCGSTAMP_NEW
> > +  IOCTL(SIOCGSTAMP_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval64)))
> > +#endif
> > +#ifdef SIOCGSTAMPNS_NEW
> > +  IOCTL(SIOCGSTAMPNS_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec64)))
> > +#endif
> >
> >IOCTL(RNDGETENTCNT, IOC_R, MK_PTR(TYPE_INT))
> >IOCTL(RNDADDTOENTCNT, IOC_W, MK_PTR(TYPE_INT))
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index b187c1281d..f13e260b02 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -37,6 +37,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> > index 7f141f699c..7830b600e7 100644
> > --- a/linux-user/syscall_defs.h
> > +++ b/linux-user/syscall_defs.h
> > @@ -750,6 +750,11 @@ struct target_pollfd {
> >
> >  #define TARGET_SIOCGSTAMP  0x8906  /* Get stamp (timeval) */
> >  #define TARGET_SIOCGSTAMPNS0x8907  /* Get stamp (timespec) */
> > +#define TARGET_SIOCGSTAMP_OLD   0x8906  /* Get stamp (timeval) */
> > +#define TARGET_SIOCGSTAMPNS_OLD 0x8907  /* Get stamp (timespec) */
> > +#define TARGET_SIOCGSTAMP_NEW   TARGET_IOC(TARGET_IOC_READ, 's', 6, 
> > sizeof(long long) + sizeof(long)) /* Get stamp (timeval64) */
> > +#define TARGET_SIOCGSTAMPNS_NEW TARGET_IOC(TARGET_IOC_READ, 's', 7, 
> > sizeof(long long) + sizeof(long)) /* Get stamp (timespec64) */
> kernel defines:
>
> #define SIOCGSTAMP_NEW   _IOR(SOCK_IOC_TYPE, 0x06, long long[2])
> #define SIOCGSTAMPNS_NEW _IOR(SOCK_IOC_TYPE, 0x07, long long[2])
>
> So it should be TARGET_IOR(0x89, 0x6, abi_llong[2])
>
> Their codes are 0x80108906 and 80108907.

Hi,
I found the discussion around this topic being almost a month old.
And related to this fedora bug [1] was closed by adding [2] which
matches [3] that was nacked in the discussion here.

Since I found nothing later (neither qemu commits nor further
discussions) I wonder if it has fallen through the cracks OR if there
was a kernel fix/change to resolve it (if that is the case a pointer
to the related kernel change would be nice)?

[1]: https://bugzilla.redhat.com/show_bug.cgi?id=1718926
[2]: 
https://src.fedoraproject.org/rpms/qemu/blob/master/f/0005-NOT-UPSTREAM-Build-fix-with-latest-kernel.patch
[3]: https://patchew.org/QEMU/20190604071915.288045-1-borntrae...@de.ibm.com/

> Thanks,
> Laurent
>


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

Re: [Qemu-devel] [PATCH v2] linux-user: fix to handle variably sized SIOCGSTAMP with new kernels

2019-06-17 Thread Laurent Vivier 
Le 17/06/2019 à 15:11, Daniel P. Berrangé a écrit :
> The SIOCGSTAMP symbol was previously defined in the
> asm-generic/sockios.h header file. QEMU sees that header
> indirectly via sys/socket.h
> 
> In linux kernel commit 0768e17073dc527ccd18ed5f96ce85f9985e9115
> the asm-generic/sockios.h header no longer defines SIOCGSTAMP.
> Instead it provides only SIOCGSTAMP_OLD, which only uses a
> 32-bit time_t on 32-bit architectures.
> 
> The linux/sockios.h header then defines SIOCGSTAMP using
> either SIOCGSTAMP_OLD or SIOCGSTAMP_NEW as appropriate. If
> SIOCGSTAMP_NEW is used, then the tv_sec field is 64-bit even
> on 32-bit architectures
> 
> To cope with this we must now define two separate syscalls,
> with corresponding old and new sizes, as well as including
> the new linux/sockios.h header.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  linux-user/ioctls.h| 15 +++
>  linux-user/syscall.c   |  1 +
>  linux-user/syscall_defs.h  |  5 +
>  linux-user/syscall_types.h |  4 
>  4 files changed, 25 insertions(+)
> 
> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> index 5e84dc7c3a..5a6d6def7e 100644
> --- a/linux-user/ioctls.h
> +++ b/linux-user/ioctls.h
> @@ -222,8 +222,23 @@
>IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
>IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
>IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
> +
> +#ifdef SIOCGSTAMP_OLD
> +  IOCTL(SIOCGSTAMP_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
> +#else
>IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
> +#endif
> +#ifdef SIOCGSTAMPNS_OLD
> +  IOCTL(SIOCGSTAMPNS_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
> +#else
>IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
> +#endif
> +#ifdef SIOCGSTAMP_NEW
> +  IOCTL(SIOCGSTAMP_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval64)))
> +#endif
> +#ifdef SIOCGSTAMPNS_NEW
> +  IOCTL(SIOCGSTAMPNS_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec64)))
> +#endif
>  
>IOCTL(RNDGETENTCNT, IOC_R, MK_PTR(TYPE_INT))
>IOCTL(RNDADDTOENTCNT, IOC_W, MK_PTR(TYPE_INT))
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index b187c1281d..f13e260b02 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -37,6 +37,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 7f141f699c..7830b600e7 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -750,6 +750,11 @@ struct target_pollfd {
>  
>  #define TARGET_SIOCGSTAMP  0x8906  /* Get stamp (timeval) */
>  #define TARGET_SIOCGSTAMPNS0x8907  /* Get stamp (timespec) */
> +#define TARGET_SIOCGSTAMP_OLD   0x8906  /* Get stamp (timeval) */
> +#define TARGET_SIOCGSTAMPNS_OLD 0x8907  /* Get stamp (timespec) */
> +#define TARGET_SIOCGSTAMP_NEW   TARGET_IOC(TARGET_IOC_READ, 's', 6, 
> sizeof(long long) + sizeof(long)) /* Get stamp (timeval64) */
> +#define TARGET_SIOCGSTAMPNS_NEW TARGET_IOC(TARGET_IOC_READ, 's', 7, 
> sizeof(long long) + sizeof(long)) /* Get stamp (timespec64) */
kernel defines:

#define SIOCGSTAMP_NEW   _IOR(SOCK_IOC_TYPE, 0x06, long long[2])
#define SIOCGSTAMPNS_NEW _IOR(SOCK_IOC_TYPE, 0x07, long long[2])

So it should be TARGET_IOR(0x89, 0x6, abi_llong[2])

Their codes are 0x80108906 and 80108907.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v2] linux-user: fix to handle variably sized SIOCGSTAMP with new kernels

2019-06-17 Thread Arnd Bergmann 
On Mon, Jun 17, 2019 at 3:11 PM Daniel P. Berrangé  wrote:
>
> The SIOCGSTAMP symbol was previously defined in the
> asm-generic/sockios.h header file. QEMU sees that header
> indirectly via sys/socket.h
>
> In linux kernel commit 0768e17073dc527ccd18ed5f96ce85f9985e9115
> the asm-generic/sockios.h header no longer defines SIOCGSTAMP.
> Instead it provides only SIOCGSTAMP_OLD, which only uses a
> 32-bit time_t on 32-bit architectures.

This is a bit misleading, as we still define SIOCGSTAMP in the
right place. asm-generic/sockios.h should not be used by normal
user space.

> The linux/sockios.h header then defines SIOCGSTAMP using
> either SIOCGSTAMP_OLD or SIOCGSTAMP_NEW as appropriate. If
> SIOCGSTAMP_NEW is used, then the tv_sec field is 64-bit even
> on 32-bit architectures
>
> To cope with this we must now define two separate syscalls,
> with corresponding old and new sizes, as well as including
> the new linux/sockios.h header.

The overall concept seems right. A few more comments on
details that may have gone wrong here. I'm not familiar with
the qemu-user implementation, so it's mostly guesswork
on my end.

>IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
>IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
>IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
> +
> +#ifdef SIOCGSTAMP_OLD
> +  IOCTL(SIOCGSTAMP_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
> +#else
>IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
> +#endif
> +#ifdef SIOCGSTAMPNS_OLD
> +  IOCTL(SIOCGSTAMPNS_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
> +#else
>IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
> +#endif
> +#ifdef SIOCGSTAMP_NEW
> +  IOCTL(SIOCGSTAMP_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval64)))
> +#endif
> +#ifdef SIOCGSTAMPNS_NEW
> +  IOCTL(SIOCGSTAMPNS_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec64)))
> +#endif

Is timespec64 a qemu type? How is it defined?

> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 7f141f699c..7830b600e7 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -750,6 +750,11 @@ struct target_pollfd {
>
>  #define TARGET_SIOCGSTAMP  0x8906  /* Get stamp (timeval) */
>  #define TARGET_SIOCGSTAMPNS0x8907  /* Get stamp (timespec) */
> +#define TARGET_SIOCGSTAMP_OLD   0x8906  /* Get stamp (timeval) */
> +#define TARGET_SIOCGSTAMPNS_OLD 0x8907  /* Get stamp (timespec) */

Note that these types are architecture specific. It seems that only
one architecture is left that has its own definitions though, so this
is only broken on arch/sh for current linux (and remains broken).

Future architectures, including 32-bit risc-v should only have the _NEW
version and not support SIOCGSTAMP_OLD at all.

When emulating risc-v user space on old kernels (pre-5.1), you may need to
translate the ioctl command and all system calls that take a 64-bit time_t into
the variants with a 32-bit time_t on the way into the kernel, and then back.

Similarly, running an old user binary on a riscv32 machine, you may
need to do the reverse translation.

> +#define TARGET_SIOCGSTAMP_NEW   TARGET_IOC(TARGET_IOC_READ, 's', 6, 
> sizeof(long long) + sizeof(long)) /* Get stamp (timeval64) */
> +#define TARGET_SIOCGSTAMPNS_NEW TARGET_IOC(TARGET_IOC_READ, 's', 7, 
> sizeof(long long) + sizeof(long)) /* Get stamp (timespec64) */

"sizeof(long long) + sizeof(long)" is not always the size of the argument to
TARGET_SIOCGSTAMP{NS}_NEW. On 32-bit architectures, the size is
two 64-bit values. sparc64 is potentially another special case, as 'struct
timeval is 'long + int' there (12 bytes).

On big-endian architectures, the nanoseconds are returned in the last
four bytes of the 16-byte structure.

>  /* Networking ioctls */
>  #define TARGET_SIOCADDRT   0x890B  /* add routing table entry */
> diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
> index b98a23b0f1..de4c5a5b6f 100644
> --- a/linux-user/syscall_types.h
> +++ b/linux-user/syscall_types.h
> @@ -20,6 +20,10 @@ STRUCT(timeval,
>  STRUCT(timespec,
> MK_ARRAY(TYPE_LONG, 2))
>
> +STRUCT(timeval64, TYPE_LONGLONG, TYPE_LONG)
> +
> +STRUCT(timespec64, TYPE_LONGLONG, TYPE_LONG)

Same here.

Arnd

Re: [Qemu-devel] [PATCH v2] linux-user: fix to handle variably sized SIOCGSTAMP with new kernels

2019-06-17 Thread no-reply 
Patchew URL: https://patchew.org/QEMU/20190617131103.1413-1-berra...@redhat.com/



Hi,

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

Subject: [Qemu-devel] [PATCH v2] linux-user: fix to handle variably sized 
SIOCGSTAMP with new kernels
Type: series
Message-id: 20190617131103.1413-1-berra...@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/20190615154352.26824-1-phi...@redhat.com -> 
patchew/20190615154352.26824-1-phi...@redhat.com
Switched to a new branch 'test'
aeb14b5 linux-user: fix to handle variably sized SIOCGSTAMP with new kernels

=== OUTPUT BEGIN ===
ERROR: line over 90 characters
#78: FILE: linux-user/syscall_defs.h:755:
+#define TARGET_SIOCGSTAMP_NEW   TARGET_IOC(TARGET_IOC_READ, 's', 6, 
sizeof(long long) + sizeof(long)) /* Get stamp (timeval64) */

ERROR: line over 90 characters
#79: FILE: linux-user/syscall_defs.h:756:
+#define TARGET_SIOCGSTAMPNS_NEW TARGET_IOC(TARGET_IOC_READ, 's', 7, 
sizeof(long long) + sizeof(long)) /* Get stamp (timespec64) */

total: 2 errors, 0 warnings, 51 lines checked

Commit aeb14b599d8b (linux-user: fix to handle variably sized SIOCGSTAMP with 
new kernels) 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


The full log is available at
http://patchew.org/logs/20190617131103.1413-1-berra...@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[Qemu-devel] [PATCH v2] linux-user: fix to handle variably sized SIOCGSTAMP with new kernels

2019-06-17 Thread Daniel P . Berrangé 
The SIOCGSTAMP symbol was previously defined in the
asm-generic/sockios.h header file. QEMU sees that header
indirectly via sys/socket.h

In linux kernel commit 0768e17073dc527ccd18ed5f96ce85f9985e9115
the asm-generic/sockios.h header no longer defines SIOCGSTAMP.
Instead it provides only SIOCGSTAMP_OLD, which only uses a
32-bit time_t on 32-bit architectures.

The linux/sockios.h header then defines SIOCGSTAMP using
either SIOCGSTAMP_OLD or SIOCGSTAMP_NEW as appropriate. If
SIOCGSTAMP_NEW is used, then the tv_sec field is 64-bit even
on 32-bit architectures

To cope with this we must now define two separate syscalls,
with corresponding old and new sizes, as well as including
the new linux/sockios.h header.

Signed-off-by: Daniel P. Berrangé 
---
 linux-user/ioctls.h| 15 +++
 linux-user/syscall.c   |  1 +
 linux-user/syscall_defs.h  |  5 +
 linux-user/syscall_types.h |  4 
 4 files changed, 25 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 5e84dc7c3a..5a6d6def7e 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -222,8 +222,23 @@
   IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
   IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
   IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
+
+#ifdef SIOCGSTAMP_OLD
+  IOCTL(SIOCGSTAMP_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
+#else
   IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
+#endif
+#ifdef SIOCGSTAMPNS_OLD
+  IOCTL(SIOCGSTAMPNS_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
+#else
   IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
+#endif
+#ifdef SIOCGSTAMP_NEW
+  IOCTL(SIOCGSTAMP_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval64)))
+#endif
+#ifdef SIOCGSTAMPNS_NEW
+  IOCTL(SIOCGSTAMPNS_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec64)))
+#endif
 
   IOCTL(RNDGETENTCNT, IOC_R, MK_PTR(TYPE_INT))
   IOCTL(RNDADDTOENTCNT, IOC_W, MK_PTR(TYPE_INT))
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index b187c1281d..f13e260b02 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 7f141f699c..7830b600e7 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -750,6 +750,11 @@ struct target_pollfd {
 
 #define TARGET_SIOCGSTAMP  0x8906  /* Get stamp (timeval) */
 #define TARGET_SIOCGSTAMPNS0x8907  /* Get stamp (timespec) */
+#define TARGET_SIOCGSTAMP_OLD   0x8906  /* Get stamp (timeval) */
+#define TARGET_SIOCGSTAMPNS_OLD 0x8907  /* Get stamp (timespec) */
+#define TARGET_SIOCGSTAMP_NEW   TARGET_IOC(TARGET_IOC_READ, 's', 6, 
sizeof(long long) + sizeof(long)) /* Get stamp (timeval64) */
+#define TARGET_SIOCGSTAMPNS_NEW TARGET_IOC(TARGET_IOC_READ, 's', 7, 
sizeof(long long) + sizeof(long)) /* Get stamp (timespec64) */
+
 
 /* Networking ioctls */
 #define TARGET_SIOCADDRT   0x890B  /* add routing table entry */
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index b98a23b0f1..de4c5a5b6f 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -20,6 +20,10 @@ STRUCT(timeval,
 STRUCT(timespec,
MK_ARRAY(TYPE_LONG, 2))
 
+STRUCT(timeval64, TYPE_LONGLONG, TYPE_LONG)
+
+STRUCT(timespec64, TYPE_LONGLONG, TYPE_LONG)
+
 STRUCT(rtentry,
TYPE_ULONG, MK_STRUCT(STRUCT_sockaddr), MK_STRUCT(STRUCT_sockaddr), 
MK_STRUCT(STRUCT_sockaddr),
TYPE_SHORT, TYPE_SHORT, TYPE_ULONG, TYPE_PTRVOID, TYPE_SHORT, 
TYPE_PTRVOID,
-- 
2.21.0