Skip to contentSkip to conversation

qemu-kvm build issue on RHEL5.1

Qemu-devel Superseded Hao, Xudong 7 participants 17 messages Oct 12, 2010 – Nov 22, 2010 KVM

v1 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Hao, Xudong

author
#0 lore raw

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

#1 lore raw

(Add CC to kvm@vger)

Quoted 19 lines · Xudong

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

Quoted 2 lines

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 <[email protected]>
---
 hw/virtio-9p-local.c |    8 ++++++++
 hw/virtio-9p.c       |    9 +++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)
hw/virtio-9p-local.c +8 -0
index 57f9243..e075c27 100644
@@ -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)
hw/virtio-9p.c +9 -0
index 32fa3bc..efe5c51 100644
@@ -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) {
Signature · 1 line
1.7.3.1
author
#2 lore raw
Quoted 103 lines · Hidetoshi Seto

(Add CC to kvm@vger)

Quoted 22 lines · Xudong

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

Quoted 2 lines

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 <[email protected]>
---
 hw/virtio-9p-local.c |    8 ++++++++
 hw/virtio-9p.c       |    9 +++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)
hw/virtio-9p-local.c +7 -0
index 57f9243..e075c27 100644
@@ -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)

hw/virtio-9p.c +2 -0
index 32fa3bc..efe5c51 100644
@@ -1393,6 +1393,15 @@ out:
     qemu_free(vs);
 }
 
+#ifndef CONFIG_UTIMENSAT
+#ifndef UTIME_NOW

hw/virtio-9p.c out: · line 1407

Quoted 10 lines
hw/virtio-9p.c +7 -0
+# 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

#3 lore raw
In reply to Hidetoshi Seto
Quoted 22 lines · Hidetoshi Seto

(Add CC to kvm@vger)

Quoted 19 lines · Xudong

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

Quoted 7 lines
Quoted 2 lines

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?

#4 lore raw
Quoted 27 lines · Blue Swirl
Quoted 25 lines · Hidetoshi Seto

(Add CC to kvm@vger)

Quoted 21 lines · Xudong

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

Quoted 8 lines
Quoted 4 lines

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?

Quoted 2 lines

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

#5 lore raw
Quoted 37 lines · * Hidetoshi Seto ([email protected]) wrote:
Quoted 27 lines · Blue Swirl
Quoted 25 lines · Hidetoshi Seto

(Add CC to kvm@vger)

Quoted 21 lines · Xudong

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

Quoted 8 lines
Quoted 4 lines

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.

Quoted 6 lines

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

#6 lore raw
Quoted 45 lines · Chris Wright
Quoted 37 lines · * Hidetoshi Seto ([email protected]) wrote:
Quoted 27 lines · Blue Swirl
Quoted 25 lines · Hidetoshi Seto

(Add CC to kvm@vger)

Quoted 21 lines · Xudong

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

Quoted 8 lines
Quoted 4 lines

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.

Quoted 6 lines

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.

Quoted 8 lines

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

#7 lore raw

PATCH 1/1 virtio-9p: fix build on !CONFIG_UTIMENSAT v2

In reply to Chris Wright

This patch introduce a fallback mechanism for old systems that do not support utimensat. This will fix build failure with following warnings:

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 <[email protected]>
---
 hw/virtio-9p-local.c |   32 ++++++++++++++++++++++++++++++--
 hw/virtio-9p.h       |   10 ++++++++++
 2 files changed, 40 insertions(+), 2 deletions(-)
hw/virtio-9p-local.c +30 -2
index 0d52020..7811d2c 100644
@@ -479,10 +479,38 @@ static int local_chown(FsContext *fs_ctx, const char *path, FsCred *credp)
     return -1;
 }
 
+/* TODO: relocate this to proper file, and make it more generic */
+static int qemu_utimensat(int dirfd, const char *path,
+                          const struct timespec *times, int flags)
+{
+#ifdef CONFIG_UTIMENSAT
+    return utimensat(dirfd, path, times, flags);
+#else
+    /*
+     * Fallback: use utimes() instead of utimensat().
+     * See commit 74bc02b2d2272dc88fb98d43e631eb154717f517 for known problem.
+     */
+    struct timeval tv[2];
+    int i;
+
+    for (i = 0; i < 2; i++) {
+        if (times[i].tv_nsec == UTIME_OMIT || times[i].tv_nsec == UTIME_NOW) {
+            tv[i].tv_sec = 0;
+            tv[i].tv_usec = 0;
+        } else {
+            tv[i].tv_sec = times[i].tv_sec;
+            tv[i].tv_usec = times[i].tv_nsec / 1000;
+        }
+    }
+
+    return utimes(path, &tv[0]);
+#endif
+}
+
 static int local_utimensat(FsContext *s, const char *path,
-		       const struct timespec *buf)
+                           const struct timespec *buf)
 {
-    return utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
+    return qemu_utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
 }
 
 static int local_remove(FsContext *ctx, const char *path)
hw/virtio-9p.h +10 -0
index 6c23319..d448d8a 100644
@@ -8,6 +8,16 @@
 
 #include "file-op-9p.h"
 
+/* TODO: relocate this to proper file */
+#ifndef CONFIG_UTIMENSAT
+#ifndef UTIME_NOW
+# define UTIME_NOW     ((1l << 30) - 1l)
+#endif
+#ifndef UTIME_OMIT
+# define UTIME_OMIT    ((1l << 30) - 2l)
+#endif
+#endif
+
 /* The feature bitmap for virtio 9P */
 /* The mount point is specified in a config variable */
 #define VIRTIO_9P_MOUNT_TAG 0
Signature · 1 line
1.7.3.1
#8 lore raw

Re: [Qemu-devel] [PATCH] virtio-9p: fix build on !CONFIG_UTIMENSAT v2

Quoted 31 lines

This patch introduce a fallback mechanism for old systems that do not support utimensat. This will fix build failure with following warnings:

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 <[email protected]>
---
 hw/virtio-9p-local.c |   32 ++++++++++++++++++++++++++++++--
 hw/virtio-9p.h       |   10 ++++++++++
 2 files changed, 40 insertions(+), 2 deletions(-)
hw/virtio-9p-local.c +0 -0
index 0d52020..7811d2c 100644

hw/virtio-9p-local.c static int local_chown(FsContext *fs_ctx, const char · line 485

Quoted 7 lines
hw/virtio-9p-local.c +4 -0
@@ -479,10 +479,38 @@ static int local_chown(FsContext *fs_ctx, const char
 *path, FsCred *credp) return -1;
 }
 
+/* TODO: relocate this to proper file, and make it more generic */
+static int qemu_utimensat(int dirfd, const char *path,
+                          const struct timespec *times, int flags)
+{

IMHO, this code can be moved to cutils.c

Quoted 13 lines
hw/virtio-9p-local.c +6 -0
+#ifdef CONFIG_UTIMENSAT
+    return utimensat(dirfd, path, times, flags);
+#else
+    /*
+     * Fallback: use utimes() instead of utimensat().
+     * See commit 74bc02b2d2272dc88fb98d43e631eb154717f517 for known

problem. + */

hw/virtio-9p-local.c +5 -0
+    struct timeval tv[2];
+    int i;
+
+    for (i = 0; i < 2; i++) {
+        if (times[i].tv_nsec == UTIME_OMIT || times[i].tv_nsec ==

UTIME_NOW) { + tv[i].tv_sec = 0;

hw/virtio-9p-local.c static int local_chown(FsContext *fs_ctx, const char *path, FsCred *credp) · line 507

Quoted 9 lines
hw/virtio-9p-local.c +9 -0
+            tv[i].tv_usec = 0;
+        } else {
+            tv[i].tv_sec = times[i].tv_sec;
+            tv[i].tv_usec = times[i].tv_nsec / 1000;
+        }
+    }
+
+    return utimes(path, &tv[0]);
+#endif

The idea of introducing utimensat was to avoid resetting atime to 1970-01-01 05:30:00 (utime does not give option to not change atime). But as per utimes man page, if any of the time field is 0, it would be set to current time. As per stat man page, truncate will not update atime, only mtime will be updated.

#9 lore raw

Re: [Qemu-devel] [PATCH] virtio-9p: fix build on !CONFIG_UTIMENSAT v2

Quoted 34 lines · M. Mohan Kumar
Quoted 34 lines

This patch introduce a fallback mechanism for old systems that do not support utimensat. This will fix build failure with following warnings:

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 <[email protected]>
---
 hw/virtio-9p-local.c |   32 ++++++++++++++++++++++++++++++--
 hw/virtio-9p.h       |   10 ++++++++++
 2 files changed, 40 insertions(+), 2 deletions(-)
hw/virtio-9p-local.c +0 -0
index 0d52020..7811d2c 100644
@@ -479,10 +479,38 @@ static int local_chown(FsContext *fs_ctx, const char
 *path, FsCred *credp) return -1;
 }
Quoted 6 lines

hw/virtio-9p-local.c static int local_chown(FsContext *fs_ctx, const char *path, FsCred *credp) · line 485

Quoted 4 lines
hw/virtio-9p-local.c +4 -0
+/* TODO: relocate this to proper file, and make it more generic */
+static int qemu_utimensat(int dirfd, const char *path,
+                          const struct timespec *times, int flags)
+{

IMHO, this code can be moved to cutils.c

It's not a C utility function, so it really belongs in oslib-posix.c, but otherwise I agree. This is emulation of a C library function, it shouldn't be in the 9p code.

Cheers,
Jes

#10 lore raw

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

In reply to Hidetoshi Seto

hw/virtio-9p-local.c static int local_chown(FsContext *fs_ctx, const char *path, FsCred *credp) · line 499

Quoted 11 lines · * Hidetoshi Seto ([email protected]) wrote:
hw/virtio-9p-local.c +11 -0
+    /*
+     * Fallback: use utimes() instead of utimensat().
+     * See commit 74bc02b2d2272dc88fb98d43e631eb154717f517 for known problem.
+     */
+    struct timeval tv[2];
+    int i;
+
+    for (i = 0; i < 2; i++) {
+        if (times[i].tv_nsec == UTIME_OMIT || times[i].tv_nsec == UTIME_NOW) {
+            tv[i].tv_sec = 0;
+            tv[i].tv_usec = 0;

I don't think this is accurate in either case. It will set the atime, mtime, or both to 0.

For UTIME_NOW (in both) we'd simply pass NULL to utimes(2). For UTIME_OMIT (in both) we'd simply skip the call to utimes(2) altogether.

The harder part is a mixed mode (i.e. the truncate fix mentioned in the above commit). I think the only way to handle UTIME_NOW in one is to call gettimeofday (or clock_gettime for better resolution) to find out what current time is. And for UTIME_OMIT call stat to find out what the current setting is and reset to that value. Both of those cases can possibly zero out the extra precision (providing only seconds resolution).

thanks,
-chris

#11 lore raw

Re: [Qemu-devel] Re: [PATCH] virtio-9p: fix build on !CONFIG_UTIMENSAT v2

Quoted 19 lines · Chris Wright

hw/virtio-9p-local.c static int local_chown(FsContext *fs_ctx, const char *path, FsCred *credp) · line 499

Quoted 11 lines · * Hidetoshi Seto ([email protected]) wrote:
hw/virtio-9p-local.c +11 -0
+    /*
+     * Fallback: use utimes() instead of utimensat().
+     * See commit 74bc02b2d2272dc88fb98d43e631eb154717f517 for known problem.
+     */
+    struct timeval tv[2];
+    int i;
+
+    for (i = 0; i < 2; i++) {
+        if (times[i].tv_nsec == UTIME_OMIT || times[i].tv_nsec == UTIME_NOW) {
+            tv[i].tv_sec = 0;
+            tv[i].tv_usec = 0;

I don't think this is accurate in either case. It will set the atime, mtime, or both to 0.

For UTIME_NOW (in both) we'd simply pass NULL to utimes(2). For UTIME_OMIT (in both) we'd simply skip the call to utimes(2) altogether.

Quoted 7 lines

The harder part is a mixed mode (i.e. the truncate fix mentioned in the above commit). I think the only way to handle UTIME_NOW in one is to call gettimeofday (or clock_gettime for better resolution) to find out what current time is. And for UTIME_OMIT call stat to find out what the current setting is and reset to that value. Both of those cases can possibly zero out the extra precision (providing only seconds resolution).

Thank you for comments!
I'll post an updated one soon.

Thanks,
H.Seto

#12 lore raw

PATCH v3 1/1 virtio-9p: fix build on !CONFIG_UTIMENSAT

This patch introduce a fallback mechanism for old systems that do not support utimensat(). This fix build failure with following warnings:

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)

v3:
- Use better alternative handling for UTIME_NOW/OMIT
- Move qemu_utimensat() to cutils.c
V2:
- Introduce qemu_utimensat()

Signed-off-by Hidetoshi Seto <[email protected]>
---
 cutils.c             |   43 +++++++++++++++++++++++++++++++++++++++++++
 hw/virtio-9p-local.c |    4 ++--
 qemu-common.h        |   10 ++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)
cutils.c +43 -0
index 536ee93..3c18941 100644
@@ -288,3 +288,46 @@ int fcntl_setfl(int fd, int flag)
 }
 #endif
 
+int qemu_utimensat(int dirfd, const char *path, const struct timespec *times,
+                   int flags)
+{
+#ifdef CONFIG_UTIMENSAT
+    return utimensat(dirfd, path, times, flags);
+#else
+    /* Fallback: use utimes() instead of utimensat() */
+    struct timeval tv[2], tv_now;
+    struct stat st;
+    int i;
+
+    /* happy if special cases */
+    if (times[0].tv_nsec == UTIME_OMIT && times[1].tv_nsec == UTIME_OMIT) {
+        return 0;
+    }
+    if (times[0].tv_nsec == UTIME_NOW && times[1].tv_nsec == UTIME_NOW) {
+        return utimes(path, NULL);
+    }
+
+    /* prepare for hard cases */
+    if (times[0].tv_nsec == UTIME_NOW || times[1].tv_nsec == UTIME_NOW) {
+        gettimeofday(&tv_now, NULL);
+    }
+    if (times[0].tv_nsec == UTIME_OMIT || times[1].tv_nsec == UTIME_OMIT) {
+        stat(path, &st);
+    }
+
+    for (i = 0; i < 2; i++) {
+        if (times[i].tv_nsec == UTIME_NOW) {
+            tv[i].tv_sec = tv_now.tv_sec;
+            tv[i].tv_usec = 0;
+        } else if (times[i].tv_nsec == UTIME_OMIT) {
+            tv[i].tv_sec = (i == 0) ? st.st_atime : st.st_mtime;
+            tv[i].tv_usec = 0;
+        } else {
+            tv[i].tv_sec = times[i].tv_sec;
+            tv[i].tv_usec = times[i].tv_nsec / 1000;
+        }
+    }
+
+    return utimes(path, &tv[0]);
+#endif
+}
hw/virtio-9p-local.c +2 -2
index 0d52020..41603ea 100644
@@ -480,9 +480,9 @@ 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)
+                           const struct timespec *buf)
 {
-    return utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
+    return qemu_utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
 }
 
 static int local_remove(FsContext *ctx, const char *path)
qemu-common.h +10 -0
index 2fbc27f..7fe4c16 100644
@@ -146,6 +146,16 @@ time_t mktimegm(struct tm *tm);
 int qemu_fls(int i);
 int qemu_fdatasync(int fd);
 int fcntl_setfl(int fd, int flag);
+#ifndef CONFIG_UTIMENSAT
+#ifndef UTIME_NOW
+# define UTIME_NOW     ((1l << 30) - 1l)
+#endif
+#ifndef UTIME_OMIT
+# define UTIME_OMIT    ((1l << 30) - 2l)
+#endif
+#endif
+int qemu_utimensat(int dirfd, const char *path, const struct timespec *times,
+    int flags);
 
 /* path.c */
 void init_paths(const char *prefix);
Signature · 1 line
1.7.3.1
Acked-by on v3 1/1
#13 lore raw

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

Quoted 17 lines · * Hidetoshi Seto ([email protected]) wrote:

This patch introduce a fallback mechanism for old systems that do not support utimensat(). This fix build failure with following warnings:

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)

Quoted 7 lines

v3:
- Use better alternative handling for UTIME_NOW/OMIT
- Move qemu_utimensat() to cutils.c
V2:
- Introduce qemu_utimensat()

Signed-off-by Hidetoshi Seto <[email protected]>

Looks good to me (no strong opinion on the cutils vs oslib-posix that Jes mentioned).

Acked-by Chris Wright <[email protected]>
Acked-by on v3 1/1
#14 lore raw

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

In reply to Hidetoshi Seto
Quoted 20 lines

This patch introduce a fallback mechanism for old systems that do not support utimensat(). This fix build failure with following warnings:

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)

Quoted 7 lines

v3:
- Use better alternative handling for UTIME_NOW/OMIT
- Move qemu_utimensat() to cutils.c
V2:
- Introduce qemu_utimensat()

Signed-off-by Hidetoshi Seto <[email protected]>

Looks good to me.

Acked-by M. Mohan Kumar <[email protected]>
Applied
#15 lore raw

Re: [Qemu-devel] [PATCH v3] virtio-9p: fix build on !CONFIG_UTIMENSAT

In reply to Hidetoshi Seto
Quoted 17 lines · Hidetoshi Seto

This patch introduce a fallback mechanism for old systems that do not support utimensat(). This fix build failure with following warnings:

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)

Quoted 8 lines

v3:
- Use better alternative handling for UTIME_NOW/OMIT
- Move qemu_utimensat() to cutils.c
V2:
- Introduce qemu_utimensat()

Signed-off-by Hidetoshi Seto<[email protected]>

Applied. Thanks.

Regards,

Anthony Liguori

Quoted 95 lines
---
  cutils.c             |   43 +++++++++++++++++++++++++++++++++++++++++++
  hw/virtio-9p-local.c |    4 ++--
  qemu-common.h        |   10 ++++++++++
  3 files changed, 55 insertions(+), 2 deletions(-)
cutils.c +43 -0
index 536ee93..3c18941 100644
@@ -288,3 +288,46 @@ int fcntl_setfl(int fd, int flag)
  }
  #endif
 
+int qemu_utimensat(int dirfd, const char *path, const struct timespec *times,
+                   int flags)
+{
+#ifdef CONFIG_UTIMENSAT
+    return utimensat(dirfd, path, times, flags);
+#else
+    /* Fallback: use utimes() instead of utimensat() */
+    struct timeval tv[2], tv_now;
+    struct stat st;
+    int i;
+
+    /* happy if special cases */
+    if (times[0].tv_nsec == UTIME_OMIT&&  times[1].tv_nsec == UTIME_OMIT) {
+        return 0;
+    }
+    if (times[0].tv_nsec == UTIME_NOW&&  times[1].tv_nsec == UTIME_NOW) {
+        return utimes(path, NULL);
+    }
+
+    /* prepare for hard cases */
+    if (times[0].tv_nsec == UTIME_NOW || times[1].tv_nsec == UTIME_NOW) {
+        gettimeofday(&tv_now, NULL);
+    }
+    if (times[0].tv_nsec == UTIME_OMIT || times[1].tv_nsec == UTIME_OMIT) {
+        stat(path,&st);
+    }
+
+    for (i = 0; i<  2; i++) {
+        if (times[i].tv_nsec == UTIME_NOW) {
+            tv[i].tv_sec = tv_now.tv_sec;
+            tv[i].tv_usec = 0;
+        } else if (times[i].tv_nsec == UTIME_OMIT) {
+            tv[i].tv_sec = (i == 0) ? st.st_atime : st.st_mtime;
+            tv[i].tv_usec = 0;
+        } else {
+            tv[i].tv_sec = times[i].tv_sec;
+            tv[i].tv_usec = times[i].tv_nsec / 1000;
+        }
+    }
+
+    return utimes(path,&tv[0]);
+#endif
+}
hw/virtio-9p-local.c +2 -2
index 0d52020..41603ea 100644
@@ -480,9 +480,9 @@ 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)
+                           const struct timespec *buf)
  {
-    return utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
+    return qemu_utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
  }
 
  static int local_remove(FsContext *ctx, const char *path)
qemu-common.h +10 -0
index 2fbc27f..7fe4c16 100644
@@ -146,6 +146,16 @@ time_t mktimegm(struct tm *tm);
  int qemu_fls(int i);
  int qemu_fdatasync(int fd);
  int fcntl_setfl(int fd, int flag);
+#ifndef CONFIG_UTIMENSAT
+#ifndef UTIME_NOW
+# define UTIME_NOW     ((1l<<  30) - 1l)
+#endif
+#ifndef UTIME_OMIT
+# define UTIME_OMIT    ((1l<<  30) - 2l)
+#endif
+#endif
+int qemu_utimensat(int dirfd, const char *path, const struct timespec *times,
+    int flags);
 
  /* path.c */
  void init_paths(const char *prefix);
#16 lore raw

Re: [Qemu-devel] [PATCH v3] virtio-9p: fix build on !CONFIG_UTIMENSAT

Quoted 30 lines · Anthony Liguori
Quoted 29 lines · Hidetoshi Seto

This patch introduce a fallback mechanism for old systems that do not support utimensat(). This fix build failure with following warnings:

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)

v3:
- Use better alternative handling for UTIME_NOW/OMIT
- Move qemu_utimensat() to cutils.c
V2:
- Introduce qemu_utimensat()

Quoted 8 lines
Quoted 2 lines
Signed-off-by Hidetoshi Seto<[email protected]>

Applied. Thanks.

Regards,

Anthony Liguori

Anthony,

Did you actually apply this one? I don't see it in the git tree.

However if you did, that was a mistake, the qemu_utimensat() should not have gone into cutils.c as I pointed out earlier, it is inconsistent.

Cheers,
Jes