Re: [PATCH] Added timestamps to storage volumes

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

 



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

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]