Message ID | 4CB56715.7080605@jp.fujitsu.com |
---|---|
State | New |
Headers | show |
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
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?
(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
* 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
(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
===== [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) {