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

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



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
>  vsnprintf
>  waitpid
>  warnings
> +stat-time
>  '

Insert in sorted order.


> @@ -172,6 +177,14 @@
>          contains the MAC (eg SELinux) label string.
>          <span class="since">Since 0.4.1</span>
>        </dd>
> +      <dt><code>timestamps</code></dt>
> +      <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'>
> +    <optional>
> +      <element name='timestamps'>
> +        <optional>
> +          <element name='atime'>
> +            <data type="string">
> +              <param name="pattern">[0-9]+\.[0-9]+</param>
> +            </data>

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'/>
  </element>

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

> +++ b/src/conf/storage_conf.c
> @@ -1277,6 +1277,24 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options,
>  
>      virBufferAddLit(buf,"    </permissions>\n");
>  
> +    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:

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

-- 
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]