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

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



On 10.07.2012 17:36, Eric Blake wrote:
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:
Ok, that sounds good. But what exactly does 'nearly' mean?
#include "stat-time.h"
...

atime = get_stat_atime(sb);
mtime = get_stat_mtime(sb);
ctime = get_stat_mtime(sb);
Of course, you are absolutely right. It is much cleaner to use stat-time.
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).
So any idea how long this will take?
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 */
}



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