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

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



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 redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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