[libvirt] [PATCH] Added timestamps to storage volumes

Hendrik Schwartke hendrik at os-t.de
Wed Jul 11 07:12:14 UTC 2012


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 */
> }
>




More information about the libvir-list mailing list