| Message ID | 4CD3A513.2010102@jp.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
> 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 <seto.hidetoshi@jp.fujitsu.com> > --- > hw/virtio-9p-local.c | 32 ++++++++++++++++++++++++++++++-- > hw/virtio-9p.h | 10 ++++++++++ > 2 files changed, 40 insertions(+), 2 deletions(-) > > diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c > index 0d52020..7811d2c 100644 > --- a/hw/virtio-9p-local.c > +++ b/hw/virtio-9p-local.c > @@ -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 > +#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 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.
On 11/08/10 07:44, M. Mohan Kumar wrote: >> 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 <seto.hidetoshi@jp.fujitsu.com> >> --- >> hw/virtio-9p-local.c | 32 ++++++++++++++++++++++++++++++-- >> hw/virtio-9p.h | 10 ++++++++++ >> 2 files changed, 40 insertions(+), 2 deletions(-) >> >> diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c >> index 0d52020..7811d2c 100644 >> --- a/hw/virtio-9p-local.c >> +++ b/hw/virtio-9p-local.c >> @@ -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 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
* Hidetoshi Seto (seto.hidetoshi@jp.fujitsu.com) wrote: > + /* > + * 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
(2010/11/14 14:58), Chris Wright wrote: > * Hidetoshi Seto (seto.hidetoshi@jp.fujitsu.com) wrote: >> + /* >> + * 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). Thank you for comments! I'll post an updated one soon. Thanks, H.Seto
diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c index 0d52020..7811d2c 100644 --- a/hw/virtio-9p-local.c +++ b/hw/virtio-9p-local.c @@ -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) diff --git a/hw/virtio-9p.h b/hw/virtio-9p.h index 6c23319..d448d8a 100644 --- a/hw/virtio-9p.h +++ b/hw/virtio-9p.h @@ -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
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 <seto.hidetoshi@jp.fujitsu.com> --- hw/virtio-9p-local.c | 32 ++++++++++++++++++++++++++++++-- hw/virtio-9p.h | 10 ++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-)