diff mbox

qemu-kvm build issue on RHEL5.1

Message ID 4CB56715.7080605@jp.fujitsu.com
State New
Headers show

Commit Message

Hidetoshi Seto Oct. 13, 2010, 8 a.m. UTC 
(Add CC to kvm@vger)

(2010/10/12 10:52), Hao, Xudong wrote:
> Hi, 
> Currently qemu-kvm build fail on RHEL5 with gcc 4.1.2, build can pass on Fedora11 with gcc 4.4.1, can anybody look on RHEL5 system?
> 
> Gcc: 4.1.2
> system: RHEL5.1
> qemu-kvm: 85566812a4f8cae721fea0224e05a7e75c08c5dd
> 
> ...
>   LINK  qemu-img
>   LINK  qemu-io
>   CC    libhw64/virtio-9p-local.o
> cc1: warnings being treated as errors
> /home/source/qemu-kvm/hw/virtio-9p-local.c: In function 'local_utimensat':
> /home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: implicit declaration of function 'utimensat'
> /home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: nested extern declaration of 'utimensat'
> make[1]: *** [virtio-9p-local.o] Error 1
> make: *** [subdir-libhw64] Error 2
> 
> 
> Best Regards,
> Xudong Hao

It seems that this issue is caused by the old glibc.
Though I don't know well about virtio-9p and suppose there
should be better fix, I confirmed that following change
removed the warnings.

Thanks,
H.Seto

Comments

Hao, Xudong Oct. 13, 2010, 8:13 a.m. UTC | #1 
Hidetoshi Seto wrote:
> (Add CC to kvm@vger)
> 
> (2010/10/12 10:52), Hao, Xudong wrote:
>> Hi,
>> Currently qemu-kvm build fail on RHEL5 with gcc 4.1.2, build can
>> pass on Fedora11 with gcc 4.4.1, can anybody look on RHEL5 system? 
>> 
>> Gcc: 4.1.2
>> system: RHEL5.1
>> qemu-kvm: 85566812a4f8cae721fea0224e05a7e75c08c5dd
>> 
>> ...
>>   LINK  qemu-img
>>   LINK  qemu-io
>>   CC    libhw64/virtio-9p-local.o
>> cc1: warnings being treated as errors
>> /home/source/qemu-kvm/hw/virtio-9p-local.c: In function
>> 'local_utimensat': /home/source/qemu-kvm/hw/virtio-9p-local.c:479:
>> warning: implicit declaration of function 'utimensat'
>> /home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: nested
>> extern declaration of 'utimensat'  
>> make[1]: *** [virtio-9p-local.o] Error 1
>> make: *** [subdir-libhw64] Error 2
>> 
>> 
>> Best Regards,
>> Xudong Hao
> 
> It seems that this issue is caused by the old glibc.
> Though I don't know well about virtio-9p and suppose there
> should be better fix, I confirmed that following change
> removed the warnings.
> 
> Thanks,
> H.Seto
> 
> =====
> 
> [PATCH] virtio-9p: fix build on !CONFIG_UTIMENSAT
> 
> This removes following warnings on RHEL5, which has utimensat syscall
> but has old glibc that doesn't have support for it:
> 
> hw/virtio-9p-local.c: In function 'local_utimensat':
> hw/virtio-9p-local.c:479: warning: implicit declaration of function
> 'utimensat' hw/virtio-9p-local.c:479: warning: nested extern
> declaration of 'utimensat' 
> 
> and
> 
> hw/virtio-9p.c: In function 'v9fs_setattr_post_chmod':
> hw/virtio-9p.c:1410: error: 'UTIME_NOW' undeclared (first use in this
> function) hw/virtio-9p.c:1410: error: (Each undeclared identifier is
> reported only once hw/virtio-9p.c:1410: error: for each function it
> appears in.) 
> hw/virtio-9p.c:1413: error: 'UTIME_OMIT' undeclared (first use in
> this function) hw/virtio-9p.c: In function 'v9fs_wstat_post_chmod':
> hw/virtio-9p.c:2905: error: 'UTIME_OMIT' undeclared (first use in
> this function) 
> 
> Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
> ---
>  hw/virtio-9p-local.c |    8 ++++++++
>  hw/virtio-9p.c       |    9 +++++++++
>  2 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
> index 57f9243..e075c27 100644
> --- a/hw/virtio-9p-local.c
> +++ b/hw/virtio-9p-local.c
> @@ -18,6 +18,9 @@
>  #include <sys/socket.h>
>  #include <sys/un.h>
>  #include <attr/xattr.h>
> +#ifndef CONFIG_UTIMENSAT
> +#include <syscall.h>
> +#endif
> 
>  static const char *rpath(FsContext *ctx, const char *path)
>  {
> @@ -476,7 +479,12 @@ static int local_chown(FsContext *fs_ctx, const
>  char *path, FsCred *credp) static int local_utimensat(FsContext *s,
>  		       const char *path, const struct timespec *buf)
>  {
> +#ifndef CONFIG_UTIMENSAT
> +    return syscall(SYS_utimensat, AT_FDCWD, rpath(s, path), buf,
> +                   AT_SYMLINK_NOFOLLOW);
> +#else
>      return utimensat(AT_FDCWD, rpath(s, path), buf,
> AT_SYMLINK_NOFOLLOW); +#endif
>  }
> 
>  static int local_remove(FsContext *ctx, const char *path)
> diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
> index 32fa3bc..efe5c51 100644
> --- a/hw/virtio-9p.c
> +++ b/hw/virtio-9p.c
> @@ -1393,6 +1393,15 @@ out:
>      qemu_free(vs);
>  }
> 
> +#ifndef CONFIG_UTIMENSAT
> +#ifndef UTIME_NOW
> +# define UTIME_NOW	((1l << 30) - 1l)
> +#endif
> +#ifndef UTIME_OMIT
> +# define UTIME_OMIT	((1l << 30) - 2l)
> +#endif
> +#endif
> +
>  static void v9fs_setattr_post_chmod(V9fsState *s, V9fsSetattrState
>  *vs, int err) {
>      if (err == -1) {

Seto, your patch works fine for me, verified on my RHEL5 system.

Thanks,
Xudong
Blue Swirl Oct. 13, 2010, 7:11 p.m. UTC | #2 
On Wed, Oct 13, 2010 at 8:00 AM, Hidetoshi Seto
<seto.hidetoshi@jp.fujitsu.com> wrote:
> (Add CC to kvm@vger)
>
> (2010/10/12 10:52), Hao, Xudong wrote:
>> Hi,
>> Currently qemu-kvm build fail on RHEL5 with gcc 4.1.2, build can pass on Fedora11 with gcc 4.4.1, can anybody look on RHEL5 system?
>>
>> Gcc: 4.1.2
>> system: RHEL5.1
>> qemu-kvm: 85566812a4f8cae721fea0224e05a7e75c08c5dd
>>
>> ...
>>   LINK  qemu-img
>>   LINK  qemu-io
>>   CC    libhw64/virtio-9p-local.o
>> cc1: warnings being treated as errors
>> /home/source/qemu-kvm/hw/virtio-9p-local.c: In function 'local_utimensat':
>> /home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: implicit declaration of function 'utimensat'
>> /home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: nested extern declaration of 'utimensat'
>> make[1]: *** [virtio-9p-local.o] Error 1
>> make: *** [subdir-libhw64] Error 2
>>
>>
>> Best Regards,
>> Xudong Hao
>
> It seems that this issue is caused by the old glibc.
> Though I don't know well about virtio-9p and suppose there
> should be better fix, I confirmed that following change
> removed the warnings.

But then the system call will be made blindly without checking if the
kernel supports utimensat(). At the minimum, there should be a sane
response to ENOSYS error.

What happens if the system headers do not define SYS_utimensat?
Hidetoshi Seto Oct. 14, 2010, 12:33 a.m. UTC | #3 
(2010/10/14 4:11), Blue Swirl wrote:
> On Wed, Oct 13, 2010 at 8:00 AM, Hidetoshi Seto
> <seto.hidetoshi@jp.fujitsu.com> wrote:
>> (Add CC to kvm@vger)
>>
>> (2010/10/12 10:52), Hao, Xudong wrote:
>>> Hi,
>>> Currently qemu-kvm build fail on RHEL5 with gcc 4.1.2, build can pass on Fedora11 with gcc 4.4.1, can anybody look on RHEL5 system?
>>>
>>> Gcc: 4.1.2
>>> system: RHEL5.1
>>> qemu-kvm: 85566812a4f8cae721fea0224e05a7e75c08c5dd
>>>
>>> ...
>>>   LINK  qemu-img
>>>   LINK  qemu-io
>>>   CC    libhw64/virtio-9p-local.o
>>> cc1: warnings being treated as errors
>>> /home/source/qemu-kvm/hw/virtio-9p-local.c: In function 'local_utimensat':
>>> /home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: implicit declaration of function 'utimensat'
>>> /home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: nested extern declaration of 'utimensat'
>>> make[1]: *** [virtio-9p-local.o] Error 1
>>> make: *** [subdir-libhw64] Error 2
>>>
>>>
>>> Best Regards,
>>> Xudong Hao
>>
>> It seems that this issue is caused by the old glibc.
>> Though I don't know well about virtio-9p and suppose there
>> should be better fix, I confirmed that following change
>> removed the warnings.
> 
> But then the system call will be made blindly without checking if the
> kernel supports utimensat(). At the minimum, there should be a sane
> response to ENOSYS error.

Yes. But I'm not sure how this virtio-9p should behave if there is
no utimensat.  I think it will be better to fix this warning first
to allow fellows using RHEL5 to restart contribute on qemu-kvm,
and change this issue to virtio-9p specific problem to allow
specialists of virtio-9p to have discussion for fix without
bothering other developers. 

... Or is it better to put abort() here instead of syscall?

> 
> What happens if the system headers do not define SYS_utimensat?

I suppose build will fail, say, we might need incremental patch
named "fix build on RHEL4" or so.
But I have no idea, whether there could be a workaround if there
is no utimensat, whether we could provide something like wrapper
named compat_utimensat or qemu_utimensat, and/or whether it makes
sense if virtio-9p have dependency with presence of utimensat.


Thanks,
H.Seto
Chris Wright Nov. 4, 2010, 5:03 p.m. UTC | #4 
* Hidetoshi Seto (seto.hidetoshi@jp.fujitsu.com) wrote:
> (2010/10/14 4:11), Blue Swirl wrote:
> > On Wed, Oct 13, 2010 at 8:00 AM, Hidetoshi Seto
> > <seto.hidetoshi@jp.fujitsu.com> wrote:
> >> (Add CC to kvm@vger)
> >>
> >> (2010/10/12 10:52), Hao, Xudong wrote:
> >>> Hi,
> >>> Currently qemu-kvm build fail on RHEL5 with gcc 4.1.2, build can pass on Fedora11 with gcc 4.4.1, can anybody look on RHEL5 system?
> >>>
> >>> Gcc: 4.1.2
> >>> system: RHEL5.1
> >>> qemu-kvm: 85566812a4f8cae721fea0224e05a7e75c08c5dd
> >>>
> >>> ...
> >>>   LINK  qemu-img
> >>>   LINK  qemu-io
> >>>   CC    libhw64/virtio-9p-local.o
> >>> cc1: warnings being treated as errors
> >>> /home/source/qemu-kvm/hw/virtio-9p-local.c: In function 'local_utimensat':
> >>> /home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: implicit declaration of function 'utimensat'
> >>> /home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: nested extern declaration of 'utimensat'
> >>> make[1]: *** [virtio-9p-local.o] Error 1
> >>> make: *** [subdir-libhw64] Error 2
> >>>
> >>>
> >>> Best Regards,
> >>> Xudong Hao
> >>
> >> It seems that this issue is caused by the old glibc.
> >> Though I don't know well about virtio-9p and suppose there
> >> should be better fix, I confirmed that following change
> >> removed the warnings.
> > 
> > But then the system call will be made blindly without checking if the
> > kernel supports utimensat(). At the minimum, there should be a sane
> > response to ENOSYS error.
> 
> Yes. But I'm not sure how this virtio-9p should behave if there is
> no utimensat.  I think it will be better to fix this warning first
> to allow fellows using RHEL5 to restart contribute on qemu-kvm,
> and change this issue to virtio-9p specific problem to allow
> specialists of virtio-9p to have discussion for fix without
> bothering other developers. 

One way to workaround this is to simply not install libattr-devel
(effecitvely disabling virtio-9p).

But I agree with Blue Swirl, need a better fallback plan.  A qemu local
implementation of something like qemu_utimensat() that simply uses
glibc/kernel interface when available and falls back to using utimes()
makes sense to me.  Then the worst case is loss of resolution from ns to
us.

thanks,
-chris
Hidetoshi Seto Nov. 5, 2010, 6:32 a.m. UTC | #5 
(2010/11/05 2:03), Chris Wright wrote:
> * Hidetoshi Seto (seto.hidetoshi@jp.fujitsu.com) wrote:
>> (2010/10/14 4:11), Blue Swirl wrote:
>>> On Wed, Oct 13, 2010 at 8:00 AM, Hidetoshi Seto
>>> <seto.hidetoshi@jp.fujitsu.com> wrote:
>>>> (Add CC to kvm@vger)
>>>>
>>>> (2010/10/12 10:52), Hao, Xudong wrote:
>>>>> Hi,
>>>>> Currently qemu-kvm build fail on RHEL5 with gcc 4.1.2, build can pass on Fedora11 with gcc 4.4.1, can anybody look on RHEL5 system?
>>>>>
>>>>> Gcc: 4.1.2
>>>>> system: RHEL5.1
>>>>> qemu-kvm: 85566812a4f8cae721fea0224e05a7e75c08c5dd
>>>>>
>>>>> ...
>>>>>   LINK  qemu-img
>>>>>   LINK  qemu-io
>>>>>   CC    libhw64/virtio-9p-local.o
>>>>> cc1: warnings being treated as errors
>>>>> /home/source/qemu-kvm/hw/virtio-9p-local.c: In function 'local_utimensat':
>>>>> /home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: implicit declaration of function 'utimensat'
>>>>> /home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: nested extern declaration of 'utimensat'
>>>>> make[1]: *** [virtio-9p-local.o] Error 1
>>>>> make: *** [subdir-libhw64] Error 2
>>>>>
>>>>>
>>>>> Best Regards,
>>>>> Xudong Hao
>>>>
>>>> It seems that this issue is caused by the old glibc.
>>>> Though I don't know well about virtio-9p and suppose there
>>>> should be better fix, I confirmed that following change
>>>> removed the warnings.
>>>
>>> But then the system call will be made blindly without checking if the
>>> kernel supports utimensat(). At the minimum, there should be a sane
>>> response to ENOSYS error.
>>
>> Yes. But I'm not sure how this virtio-9p should behave if there is
>> no utimensat.  I think it will be better to fix this warning first
>> to allow fellows using RHEL5 to restart contribute on qemu-kvm,
>> and change this issue to virtio-9p specific problem to allow
>> specialists of virtio-9p to have discussion for fix without
>> bothering other developers. 
> 
> One way to workaround this is to simply not install libattr-devel
> (effecitvely disabling virtio-9p).
> 
> But I agree with Blue Swirl, need a better fallback plan.  A qemu local
> implementation of something like qemu_utimensat() that simply uses
> glibc/kernel interface when available and falls back to using utimes()
> makes sense to me.  Then the worst case is loss of resolution from ns to
> us.

According to the commit 74bc02b2d2272dc88fb98d43e631eb154717f517, the
title "Do not reset atime" can tell us that the original motivation to
use utimensat() is not for the resolution.

Anyway, I agree to have something like qemu_utimensat().
I made a patch for the first step, and will post it next to this reply. 


Thanks,
H.Seto
diff mbox

Patch

=====

[PATCH] virtio-9p: fix build on !CONFIG_UTIMENSAT

This removes following warnings on RHEL5, which has utimensat syscall
but has old glibc that doesn't have support for it:

hw/virtio-9p-local.c: In function 'local_utimensat':
hw/virtio-9p-local.c:479: warning: implicit declaration of function 'utimensat'
hw/virtio-9p-local.c:479: warning: nested extern declaration of 'utimensat'

and

hw/virtio-9p.c: In function 'v9fs_setattr_post_chmod':
hw/virtio-9p.c:1410: error: 'UTIME_NOW' undeclared (first use in this function)
hw/virtio-9p.c:1410: error: (Each undeclared identifier is reported only once
hw/virtio-9p.c:1410: error: for each function it appears in.)
hw/virtio-9p.c:1413: error: 'UTIME_OMIT' undeclared (first use in this function)
hw/virtio-9p.c: In function 'v9fs_wstat_post_chmod':
hw/virtio-9p.c:2905: error: 'UTIME_OMIT' undeclared (first use in this function)

Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
---
 hw/virtio-9p-local.c |    8 ++++++++
 hw/virtio-9p.c       |    9 +++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
index 57f9243..e075c27 100644
--- a/hw/virtio-9p-local.c
+++ b/hw/virtio-9p-local.c
@@ -18,6 +18,9 @@ 
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <attr/xattr.h>
+#ifndef CONFIG_UTIMENSAT
+#include <syscall.h>
+#endif
 
 static const char *rpath(FsContext *ctx, const char *path)
 {
@@ -476,7 +479,12 @@  static int local_chown(FsContext *fs_ctx, const char *path, FsCred *credp)
 static int local_utimensat(FsContext *s, const char *path,
 		       const struct timespec *buf)
 {
+#ifndef CONFIG_UTIMENSAT
+    return syscall(SYS_utimensat, AT_FDCWD, rpath(s, path), buf,
+                   AT_SYMLINK_NOFOLLOW);
+#else
     return utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
+#endif
 }
 
 static int local_remove(FsContext *ctx, const char *path)
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 32fa3bc..efe5c51 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1393,6 +1393,15 @@  out:
     qemu_free(vs);
 }
 
+#ifndef CONFIG_UTIMENSAT
+#ifndef UTIME_NOW
+# define UTIME_NOW	((1l << 30) - 1l)
+#endif
+#ifndef UTIME_OMIT
+# define UTIME_OMIT	((1l << 30) - 2l)
+#endif
+#endif
+
 static void v9fs_setattr_post_chmod(V9fsState *s, V9fsSetattrState *vs, int err)
 {
     if (err == -1) {