[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

I reconsidered the way timestamps are represented. I think that an event at 100.91 happened before 100.200 is misleading. So I changed that. Furthermore I would appreciate if the timestamps are available in 0.10.0, so I splitted the patch. The first patch doesn't use stat-time and the second patches the first and does use stat-time. I would be nice if at least the first one could be commited before the next version is tagged.


On 13.07.2012 17:14, Eric Blake wrote:
On 07/13/2012 08:38 AM, Hendrik Schwartke wrote:
!!! DON'T PUSH until stat-time lgpl 3 issue is fixed
!!! To tests this change lgpl version to 3 in bootstrap.conf:176

The access, birth, modification and change times are added to
storage volumes and corresponding xml representations.
  bootstrap.conf                |    1 +
  docs/formatstorage.html.in    |   13 +++++++++++++
  docs/schemas/storagevol.rng   |   36 ++++++++++++++++++++++++++++++++++++
  src/conf/storage_conf.c       |   18 ++++++++++++++++++
  src/conf/storage_conf.h       |   13 +++++++++++++
  src/storage/storage_backend.c |    6 ++++++
  6 files changed, 87 insertions(+)

diff --git a/bootstrap.conf b/bootstrap.conf
index 9b42cbf..da0b960 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -115,6 +115,7 @@ vc-list-files
Insert in sorted order.

@@ -172,6 +177,14 @@
          contains the MAC (eg SELinux) label string.
          <span class="since">Since 0.4.1</span>
+<dd>Provides timing information about the volume. The four sub elements
since btime is omitted on Linux, maybe this would read better as 'Up to
four sub-elements are present, where'

+<code>atime</code>,<code>btime</code>,<code>ctime</code>  and<code>mtime</code>
+        hold the access, birth, change and modification time of the volume, where known.
+        The used time format is&lt;seconds&gt;.&lt;nanoseconds&gt; since the beginning
+        of the epoch.  This is a readonly attribute and is ignored when creating
+        a volume.<span class="since">Since 0.10.0</span>

+<define name='timestamps'>
+<element name='timestamps'>
+<element name='atime'>
+<data type="string">
+<param name="pattern">[0-9]+\.[0-9]+</param>
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'/>

<define name='timestamp'>
   <data type='string'>
     <param name='pattern'>[0-9]+(\.[0-9]+)?</param>

+++ b/src/conf/storage_conf.c
@@ -1277,6 +1277,24 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options,


+    virBufferAddLit(buf, "<timestamps>\n");
+    virBufferAsprintf(buf, "<atime>%llu.%ld</atime>\n",
+                      (unsigned long long) def->timestamps.atime.tv_sec,
+                      def->timestamps.atime.tv_nsec);
Eliding a sub-second suffix when tv_nsec == 0 would be easier with a
helper function:

virStorageVolTimestampFormat(virBufferPtr buf, const char *name,
                              struct timespec *ts)
     if (ts->tv_nsec<  0)
     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).

+typedef virStorageTimestamps *virStorageTimestampsPtr;
+struct _virStorageTimestamps {
+    struct timespec atime;
+    /* if btime.tv_sec == -1&&  btime.tv_nsec == -1 than
+     * birth time is unknown
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.

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]