Re: [libvirt] [PATCH] Added timestamps to storage volumes

On 07/10/2012 09:22 AM, Hendrik Schwartke wrote: > The access, modification and change times are added to storage > volumes and corresponding xml representations. > --- > docs/formatstorage.html.in | 13 +++++++++++++ > docs/schemas/storagevol.rng | 23 +++++++++++++++++++++++ > src/conf/storage_conf.c | 12 ++++++++++++ > src/conf/storage_conf.h | 9 +++++++++ > src/storage/storage_backend.c | 20 ++++++++++++++++++++ > 5 files changed, 77 insertions(+) > </dd> > +<dt><code>timestamps</code></dt> > +<dd>Provides timing information about the volume. The three sub elements > +<code>atime</code>,<code>mtime</code> and<code>ctime</code> hold the > + access, modification and respectively the change time of the volume. The Grammar: hold the access, modification, and change times of the volume, where known. no need to mention 'respectively'. > +++ b/src/conf/storage_conf.c > @@ -1272,6 +1272,18 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, > > virBufferAddLit(buf,"</permissions>\n"); > > + virBufferAddLit(buf, "<timestamps>\n"); > + virBufferAsprintf(buf, "<atime>%llu.%lu</atime>\n", > + (unsigned long long) def->timestamps.atime.tv_sec, > + def->timestamps.atime.tv_nsec); Technically, tv_nsec is a signed long, and you should be using %ld instead of %lu. > +++ b/src/storage/storage_backend.c > @@ -1156,6 +1156,9 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target, > unsigned long long *capacity) > { > struct stat sb; > + struct timespec *const atime=&target->timestamps.atime, Space on either side of '='. > > + atime->tv_sec = sb.st_atime; > + mtime->tv_sec = sb.st_mtime; > + catime->tv_sec = sb.st_ctime; > +#if _BSD_SOURCE || _SVID_SOURCE > + atime->tv_nsec = sb.st_atim.tv_nsec; Yuck. I've nearly got consensus to use the gnulib stat-time module, in which case this would instead be the much simpler:

#include "stat-time.h" ... atime = get_stat_atime(sb); mtime = get_stat_mtime(sb); ctime = get_stat_mtime(sb);

But before we can use the gnulib stat-time module, we have to update .gnulib and bootstrap.conf; and I'm holding off on the .gnulib update until after fixed Automake is available in at least Fedora 17 (right now, unless you are manually using automake 1.11.6 or newer, you are injecting a security bug into every other package that uses automake).

Also, with gnulib's stat-time module, my earlier suggestion to include birthtime would be as simple as: btime = get_state_birthtime(sb); along with filtering the display: if (btime->tv_nsec == -1) { /* birth time not available */ }