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

On 07/13/2012 08:38 AM, Hendrik Schwartke wrote: Insert in sorted order. since btime is omitted on Linux, maybe this would read better as 'Up to four sub-elements are present, where' It might be worth writing the regex to permit eliding the sub-second resolution, on file systems that only have 1 second resolution. Given that we are repeating this <data> four times, it might be worth defining it, for a shorter diff: <element name='atime'> <ref name='timestamp'/> </element> ... <define name='timestamp'> <data type='string'> <param name='pattern'>[0-9]+(\.[0-9]+)?</param> </data> </define> Eliding a sub-second suffix when tv_nsec == 0 would be easier with a helper function: void virStorageVolTimestampFormat(virBufferPtr buf, const char *name, struct timespec *ts) { if (ts->tv_nsec < 0) return; virBufferAsprintf(buf, " <%s>%llu", name, (unsigned long long) ts->tv_sec); if (ts->tv_nsec) virBufferAsprintf(buf, ".%ld", tv->tv_nsec); virBufferAsprintf(buf, "</%s>\n", name); } called as: virStorageVolTimestampFormat(buf, "atime", &def->timestamps.atime); virStorageVolTimestampFormat(buf, "atime", &def->timestamps.btime); and so on. Actually, I'd list atime, mtime, ctime, btime - in that order - rather than trying to sort the names alphabetically (that is, match typical 'struct stat' ordering). Doesn't gnulib guarantee that tv_nsec == -1 in isolation is sufficient to point out an unknown value? That is, checking tv_sec == -1 is overhead. Looking nicer. I'll have to ping upstream on gnulib about the last holdout on the relicensing of stat-time; and I'm also still waiting for the security fix in updated automake to hit Fedora. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org