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

2012-08-03 Thread Hendrik Schwartke

Thanks for your support Eric!

Hendrik


On 03.08.2012 01:14, Eric Blake wrote:

On 07/25/2012 01:43 AM, Hendrik Schwartke wrote:

The access, birth, modification and change times are added to
storage volumes and corresponding xml representations.

Listing the actual output XML in the commit message will help future
readers.


---
  bootstrap.conf|1 +
  docs/formatstorage.html.in|   16 
  docs/schemas/storagevol.rng   |   34 ++
  src/conf/storage_conf.c   |   20 
  src/conf/storage_conf.h   |   13 +
  src/storage/storage_backend.c |6 ++
  6 files changed, 90 insertions(+)

We should really update at least one test to validate our rng changes.
For that matter, 'make check' fails without at least some updates,
because your code blindly outputs  with contents of 0 for a
default-initialized struct, and with no way to parse input, the xml2xml
round-trip test can't validate our output code.


+++ b/docs/formatstorage.html.in
@@ -141,6 +141,11 @@
  0744
  

+
+1341933637.27319099

That's only 8 digits for subsecond resolution.  Are you truncating a
trailing 0, or are you missing a leading 0?  Thinking about it more,
it's easier for end users to parse a fixed-length 9-digit number with
trailing zeros and always have it be scaled correctly than it is to make
them parse an arbitrary length number and then scale it to 9 digits, so
I'd prefer leaving trailing zeros intact and only omit the subsecond
resolution when it is exactly 0, at least on output.


@@ -172,6 +177,17 @@
  contains the MAC (eg SELinux) label string.
  Since 0.4.1

+timestamps
+Provides timing information about the volume. Up to four sub-elements are
+present, whereatime,btime,ctime
+andmtime  hold the access, birth, change and modification 
time
+of the volume, where known. The used time format is
+. since the beginning of the epoch. If
+nanosecond resolution isn't supported by the host OS or filesystem 
then the
+nanoseconds part is omitted. It is also omitted when zero. This is a
+readonly attribute and is ignored when creating a volume.
+Since 0.10.0

Long lines; I reformatted to fit in 80 columns.

Technically, while  and  must be ignored (as we can't
really fake them), we could use futimens during creation to honor
  and  as a future extension, if we thought it was worth
the ability to let people create volumes with hand-controlled
timestamps.  Doesn't affect this patch, though.


+++ b/docs/schemas/storagevol.rng
@@ -63,6 +63,39 @@
  


+
+
+
+
+
+
+
+

If we want to allow creation to specify timestamps, then we should allow
  of these subelements.  In fact, I see no harm in allowing
that now.


+
+
+
+[0-9]+(\.[0-9]+)?

On output, we could enforce {9} instead of + in this regex.  But if we
ever allow input, then this is too strict (we want {0,9} to allow the
user to give a shortened form, and deal with scaling the value
appropriately on our input parse).


@@ -1277,6 +1290,13 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr 
options,

  virBufferAddLit(buf,"\n");

+virBufferAddLit(buf, "\n");
+virStorageVolTimestampFormat(buf, "atime",&def->timestamps.atime);
+virStorageVolTimestampFormat(buf, "btime",&def->timestamps.btime);
+virStorageVolTimestampFormat(buf, "ctime",&def->timestamps.ctime);
+virStorageVolTimestampFormat(buf, "mtime",&def->timestamps.mtime);

I really do think laying this out in 'struct stat' order makes more
sense; atim, mtim, ctim, then btim.  XPath notation will be able to find
it regardless of our ordering.

I don't know why we use -Waggregate-return; gcc doesn't like stat-time.h
as a result; so I had to disable it for now; maybe upstream gnulib will
let us change the signature to something that modifies a pointer
argument instead of returning an aggregate, but that's a change for down
the road.

Another nice followup patch would be adding timestamps to directory
storagepool output XML.

Everything else looked good.  Thanks again for your patience.  Here's
what I squashed in, before pushing:

diff --git i/docs/formatstorage.html.in w/docs/formatstorage.html.in
index 2a578e9..9f93db8 100644
--- i/docs/formatstorage.html.in
+++ w/docs/formatstorage.html.in
@@ -142,9 +142,9 @@
  


-1341933637.27319099
-1341930622.47245868
-1341930622.47245868
+1341933637.273190990
+1341930622.047245868
+1341930622.047245868


  ...
@@ -178,14 +178,16 @@
  Since 0.4.1

timestamps
-Provides timing informati

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

2012-08-02 Thread Eric Blake
On 07/25/2012 01:43 AM, Hendrik Schwartke wrote:
> The access, birth, modification and change times are added to
> storage volumes and corresponding xml representations.

Listing the actual output XML in the commit message will help future
readers.

> ---
>  bootstrap.conf|1 +
>  docs/formatstorage.html.in|   16 
>  docs/schemas/storagevol.rng   |   34 ++
>  src/conf/storage_conf.c   |   20 
>  src/conf/storage_conf.h   |   13 +
>  src/storage/storage_backend.c |6 ++
>  6 files changed, 90 insertions(+)

We should really update at least one test to validate our rng changes.
For that matter, 'make check' fails without at least some updates,
because your code blindly outputs  with contents of 0 for a
default-initialized struct, and with no way to parse input, the xml2xml
round-trip test can't validate our output code.

> +++ b/docs/formatstorage.html.in
> @@ -141,6 +141,11 @@
>  0744
>  
>
> +  
> +1341933637.27319099

That's only 8 digits for subsecond resolution.  Are you truncating a
trailing 0, or are you missing a leading 0?  Thinking about it more,
it's easier for end users to parse a fixed-length 9-digit number with
trailing zeros and always have it be scaled correctly than it is to make
them parse an arbitrary length number and then scale it to 9 digits, so
I'd prefer leaving trailing zeros intact and only omit the subsecond
resolution when it is exactly 0, at least on output.

> @@ -172,6 +177,17 @@
>  contains the MAC (eg SELinux) label string.
>  Since 0.4.1
>
> +  timestamps
> +  Provides timing information about the volume. Up to four 
> sub-elements are
> +present, where atime, btime, 
> ctime
> +and mtime hold the access, birth, change and 
> modification time
> +of the volume, where known. The used time format is
> +. since the beginning of the 
> epoch. If
> +nanosecond resolution isn't supported by the host OS or filesystem 
> then the
> +nanoseconds part is omitted. It is also omitted when zero. This is a
> +readonly attribute and is ignored when creating a volume.
> +Since 0.10.0

Long lines; I reformatted to fit in 80 columns.

Technically, while  and  must be ignored (as we can't
really fake them), we could use futimens during creation to honor
 and  as a future extension, if we thought it was worth
the ability to let people create volumes with hand-controlled
timestamps.  Doesn't affect this patch, though.

> +++ b/docs/schemas/storagevol.rng
> @@ -63,6 +63,39 @@
>  
>
>  
> +  
> +
> +  
> +
> +  
> +
> +  
> +

If we want to allow creation to specify timestamps, then we should allow
 of these subelements.  In fact, I see no harm in allowing
that now.

> +
> +  
> +
> +  [0-9]+(\.[0-9]+)?

On output, we could enforce {9} instead of + in this regex.  But if we
ever allow input, then this is too strict (we want {0,9} to allow the
user to give a shortened form, and deal with scaling the value
appropriately on our input parse).

> @@ -1277,6 +1290,13 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr 
> options,
>  
>  virBufferAddLit(buf,"\n");
>  
> +virBufferAddLit(buf, "\n");
> +virStorageVolTimestampFormat(buf, "atime", &def->timestamps.atime);
> +virStorageVolTimestampFormat(buf, "btime", &def->timestamps.btime);
> +virStorageVolTimestampFormat(buf, "ctime", &def->timestamps.ctime);
> +virStorageVolTimestampFormat(buf, "mtime", &def->timestamps.mtime);

I really do think laying this out in 'struct stat' order makes more
sense; atim, mtim, ctim, then btim.  XPath notation will be able to find
it regardless of our ordering.

I don't know why we use -Waggregate-return; gcc doesn't like stat-time.h
as a result; so I had to disable it for now; maybe upstream gnulib will
let us change the signature to something that modifies a pointer
argument instead of returning an aggregate, but that's a change for down
the road.

Another nice followup patch would be adding timestamps to directory
storagepool output XML.

Everything else looked good.  Thanks again for your patience.  Here's
what I squashed in, before pushing:

diff --git i/docs/formatstorage.html.in w/docs/formatstorage.html.in
index 2a578e9..9f93db8 100644
--- i/docs/formatstorage.html.in
+++ w/docs/formatstorage.html.in
@@ -142,9 +142,9 @@
 
   
   
-1341933637.27319099
-1341930622.47245868
-1341930622.47245868
+1341933637.273190990
+1341930622.047245868

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

2012-07-30 Thread Hendrik Schwartke

Hi Eric,

could you give me a short feedback on the status of my patch?

Thanks
Hendrik


On 25.07.2012 09:43, Hendrik Schwartke wrote:

The access, birth, modification and change times are added to
storage volumes and corresponding xml representations.
---
  bootstrap.conf|1 +
  docs/formatstorage.html.in|   16 
  docs/schemas/storagevol.rng   |   34 ++
  src/conf/storage_conf.c   |   20 
  src/conf/storage_conf.h   |   13 +
  src/storage/storage_backend.c |6 ++
  6 files changed, 90 insertions(+)

diff --git a/bootstrap.conf b/bootstrap.conf
index 9b42cbf..d80d92d 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -90,6 +90,7 @@ sigaction
  sigpipe
  snprintf
  socket
+stat-time
  stdarg
  stpcpy
  strchrnul
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index d0e4319..2a578e9 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -141,6 +141,11 @@
  0744
  

+
+1341933637.27319099
+1341930622.47245868
+1341930622.47245868
+

  ...

@@ -172,6 +177,17 @@
  contains the MAC (eg SELinux) label string.
  Since 0.4.1

+timestamps
+Provides timing information about the volume. Up to four sub-elements are
+present, whereatime,btime,ctime
+andmtime  hold the access, birth, change and modification 
time
+of the volume, where known. The used time format is
+. since the beginning of the epoch. If
+nanosecond resolution isn't supported by the host OS or filesystem 
then the
+nanoseconds part is omitted. It is also omitted when zero. This is a
+readonly attribute and is ignored when creating a volume.
+Since 0.10.0
+
encryption
If present, specifies how the volume is encrypted.  See
  theStorage Encryption  page
diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng
index 7a74331..f981b47 100644
--- a/docs/schemas/storagevol.rng
+++ b/docs/schemas/storagevol.rng
@@ -63,6 +63,39 @@
  


+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+[0-9]+(\.[0-9]+)?
+
+
+

  

@@ -72,6 +105,7 @@



+

  

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 36a3bb9..bc8e8be 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1241,6 +1241,19 @@ virStorageVolDefParseFile(virStoragePoolDefPtr pool,
  return virStorageVolDefParse(pool, NULL, filename);
  }

+static 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, ".%09ld", ts->tv_nsec);
+virBufferAsprintf(buf, "\n", name);
+}
+
  static int
  virStorageVolTargetDefFormat(virStorageVolOptionsPtr options,
   virBufferPtr buf,
@@ -1277,6 +1290,13 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr 
options,

  virBufferAddLit(buf,"\n");

+virBufferAddLit(buf, "\n");
+virStorageVolTimestampFormat(buf, "atime",&def->timestamps.atime);
+virStorageVolTimestampFormat(buf, "btime",&def->timestamps.btime);
+virStorageVolTimestampFormat(buf, "ctime",&def->timestamps.ctime);
+virStorageVolTimestampFormat(buf, "mtime",&def->timestamps.mtime);
+virBufferAddLit(buf, "\n");
+
  if (def->encryption) {
  virBufferAdjustIndent(buf, 4);
  if (virStorageEncryptionFormat(buf, def->encryption)<  0)
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 5733b57..b67ef64 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -46,6 +46,18 @@ struct _virStoragePerms {

  /* Storage volumes */

+typedef struct _virStorageTimestamps virStorageTimestamps;
+typedef virStorageTimestamps *virStorageTimestampsPtr;
+struct _virStorageTimestamps {
+struct timespec atime;
+/* if btime.tv_nsec == -1 then
+ * birth time is unknown
+ */
+struct timespec btime;
+struct timespec ctime;
+struct timespec mtime;
+};
+

  /*
   * How the volume's data is stored on underlying
@@ -77,6 +89,7 @@ struct _virStorageVolTarget {
  char *path;
  int format;
  virStoragePerms perms;
+virStorageTimestamps timestamps;
  int type; /* only used by disk backend for partition type */
  /* Currently used only in virStorageVolDef.target, not in .backingstore. 
*/
  virStorageEncryptionPtr encryption;
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.

[libvirt] [PATCH] Added timestamps to storage volumes

2012-07-25 Thread Hendrik Schwartke
The access, birth, modification and change times are added to
storage volumes and corresponding xml representations.
---
 bootstrap.conf|1 +
 docs/formatstorage.html.in|   16 
 docs/schemas/storagevol.rng   |   34 ++
 src/conf/storage_conf.c   |   20 
 src/conf/storage_conf.h   |   13 +
 src/storage/storage_backend.c |6 ++
 6 files changed, 90 insertions(+)

diff --git a/bootstrap.conf b/bootstrap.conf
index 9b42cbf..d80d92d 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -90,6 +90,7 @@ sigaction
 sigpipe
 snprintf
 socket
+stat-time
 stdarg
 stpcpy
 strchrnul
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index d0e4319..2a578e9 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -141,6 +141,11 @@
 0744
 
   
+  
+1341933637.27319099
+1341930622.47245868
+1341930622.47245868
+  
   
 ...
   
@@ -172,6 +177,17 @@
 contains the MAC (eg SELinux) label string.
 Since 0.4.1
   
+  timestamps
+  Provides timing information about the volume. Up to four 
sub-elements are
+present, where atime, btime, 
ctime
+and mtime hold the access, birth, change and modification 
time
+of the volume, where known. The used time format is
+. since the beginning of the epoch. 
If
+nanosecond resolution isn't supported by the host OS or filesystem 
then the
+nanoseconds part is omitted. It is also omitted when zero. This is a
+readonly attribute and is ignored when creating a volume.
+Since 0.10.0
+  
   encryption
   If present, specifies how the volume is encrypted.  See
 the Storage Encryption page
diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng
index 7a74331..f981b47 100644
--- a/docs/schemas/storagevol.rng
+++ b/docs/schemas/storagevol.rng
@@ -63,6 +63,39 @@
 
   
 
+  
+
+  
+
+  
+
+  
+
+
+  
+
+  
+
+
+  
+
+  
+
+
+  
+
+  
+
+  
+
+  
+
+  
+
+  [0-9]+(\.[0-9]+)?
+
+  
+
   
 
   
@@ -72,6 +105,7 @@
   
   
   
+  
   
 
   
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 36a3bb9..bc8e8be 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1241,6 +1241,19 @@ virStorageVolDefParseFile(virStoragePoolDefPtr pool,
 return virStorageVolDefParse(pool, NULL, filename);
 }
 
+static 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, ".%09ld", ts->tv_nsec);
+virBufferAsprintf(buf, "\n", name);
+}
+
 static int
 virStorageVolTargetDefFormat(virStorageVolOptionsPtr options,
  virBufferPtr buf,
@@ -1277,6 +1290,13 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr 
options,
 
 virBufferAddLit(buf,"\n");
 
+virBufferAddLit(buf, "\n");
+virStorageVolTimestampFormat(buf, "atime", &def->timestamps.atime);
+virStorageVolTimestampFormat(buf, "btime", &def->timestamps.btime);
+virStorageVolTimestampFormat(buf, "ctime", &def->timestamps.ctime);
+virStorageVolTimestampFormat(buf, "mtime", &def->timestamps.mtime);
+virBufferAddLit(buf, "\n");
+
 if (def->encryption) {
 virBufferAdjustIndent(buf, 4);
 if (virStorageEncryptionFormat(buf, def->encryption) < 0)
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 5733b57..b67ef64 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -46,6 +46,18 @@ struct _virStoragePerms {
 
 /* Storage volumes */
 
+typedef struct _virStorageTimestamps virStorageTimestamps;
+typedef virStorageTimestamps *virStorageTimestampsPtr;
+struct _virStorageTimestamps {
+struct timespec atime;
+/* if btime.tv_nsec == -1 then
+ * birth time is unknown
+ */
+struct timespec btime;
+struct timespec ctime;
+struct timespec mtime;
+};
+
 
 /*
  * How the volume's data is stored on underlying
@@ -77,6 +89,7 @@ struct _virStorageVolTarget {
 char *path;
 int format;
 virStoragePerms perms;
+virStorageTimestamps timestamps;
 int type; /* only used by disk backend for partition type */
 /* Currently used 

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

2012-07-20 Thread Hendrik Schwartke

On 19.07.2012 16:08, Eric Blake wrote:

On 07/19/2012 01:13 AM, Hendrik Schwartke wrote:

I reconsidered the way timestamps are represented. I think that an event
at 100.91 happened before 100.200 is misleading.

Yep, definite bug - you have to zero-pad the subsecond resolution, and
also consider whether to strip trailing zeros.


So I changed that.
Furthermore I would appreciate if the timestamps are available in
0.10.0, so I splitted the patch. The first patch doesn't use stat-time
and the second patches the first and does use stat-time.

Not necessary to do two implementations.  stat-time has now been
relicensed in gnulib, so all we are waiting on now is for updated
automake to hit Fedora 17 (it has already hit rawhide):
https://bugzilla.redhat.com/show_bug.cgi?id=838660

Wow, that was fast. Many thanks for your help.


Given the security bug in current automake, I will actively protest
releasing the next version of libvirt until fixed automake is available.

Don't worry - I plan to include this feature in the next libvirt build;
just a few more days of waiting.



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


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

2012-07-19 Thread Eric Blake
On 07/19/2012 01:13 AM, Hendrik Schwartke wrote:
> I reconsidered the way timestamps are represented. I think that an event
> at 100.91 happened before 100.200 is misleading.

Yep, definite bug - you have to zero-pad the subsecond resolution, and
also consider whether to strip trailing zeros.

> So I changed that.
> Furthermore I would appreciate if the timestamps are available in
> 0.10.0, so I splitted the patch. The first patch doesn't use stat-time
> and the second patches the first and does use stat-time.

Not necessary to do two implementations.  stat-time has now been
relicensed in gnulib, so all we are waiting on now is for updated
automake to hit Fedora 17 (it has already hit rawhide):
https://bugzilla.redhat.com/show_bug.cgi?id=838660

Given the security bug in current automake, I will actively protest
releasing the next version of libvirt until fixed automake is available.

Don't worry - I plan to include this feature in the next libvirt build;
just a few more days of waiting.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-07-19 Thread Hendrik Schwartke
I reconsidered the way timestamps are represented. I think that an event 
at 100.91 happened before 100.200 is misleading. So I changed that.
Furthermore I would appreciate if the timestamps are available in 
0.10.0, so I splitted the patch. The first patch doesn't use stat-time 
and the second patches the first and does use stat-time. I would be nice 
if at least the first one could be commited before the next version is 
tagged.


Thanks
Hendrik


On 13.07.2012 17:14, Eric Blake wrote:

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.
  Since 0.4.1

+timestamps
+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'


+atime,btime,ctime  andmtime
+hold the access, birth, change and modification time of the volume, 
where known.
+The used time format is. since the 
beginning
+of the epoch.  This is a readonly attribute and is ignored when 
creating
+a volume.Since 0.10.0



+
+
+
+
+
+
+[0-9]+\.[0-9]+
+

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  four times, it might be worth defining
it, for a shorter diff:

   
 
   

...

   
 [0-9]+(\.[0-9]+)?
   



+++ b/src/conf/storage_conf.c
@@ -1277,6 +1277,24 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr 
options,

  virBufferAddLit(buf,"\n");

+virBufferAddLit(buf, "\n");
+virBufferAsprintf(buf, "%llu.%ld\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, "\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.



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


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

2012-07-16 Thread Hendrik Schwartke

On 16.07.2012 09:45, Hendrik Schwartke wrote:

On 13.07.2012 17:14, Eric Blake wrote:

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.
Since 0.4.1

+timestamps
+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'

+atime,btime,ctime  
andmtime
+hold the access, birth, change and modification time of the 
volume, where known.
+The used time format is. 
since the beginning
+of the epoch.  This is a readonly attribute and is ignored 
when creating

+a volume.Since 0.10.0



+
+
+
+
+
+
+[0-9]+\.[0-9]+
+

It might be worth writing the regex to permit eliding the sub-second
resolution, on file systems that only have 1 second resolution.  Given
Well, the problem here is that stat-time doesn't offer a way to 
determine if sub-second resolution is available. If the system doesn't 
support it then tv_nsec is simply zero. So there is always a 
sub-second part in the timestamp and such an regex could be slightly 
misleading. I will change it anyway and add a comment to the schema.

that we are repeating this  four times, it might be worth defining
it, for a shorter diff:





...


[0-9]+(\.[0-9]+)?




+++ b/src/conf/storage_conf.c
@@ -1277,6 +1277,24 @@ 
virStorageVolTargetDefFormat(virStorageVolOptionsPtr options,


  virBufferAddLit(buf,"\n");

+virBufferAddLit(buf, "\n");
+virBufferAsprintf(buf, "%llu.%ld\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)

That's never the case. See above.
Yups, wrong line. Of course that could be the case. But again I prefer 
to check tv_sec also.

 return;
 virBufferAsprintf(buf, "<%s>%llu", name,
   (unsigned long long) ts->tv_sec);
 if (ts->tv_nsec)
That the line I wanted to comment. I'm not sure if it's such a good idea 
to omit the sub second part. Although it's very unlikely that this 
happends on systems that support tv_nsec it could be misleading.

 virBufferAsprintf(buf, ".%ld", tv->tv_nsec);
 virBufferAsprintf(buf, "\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).
Well I thought about that and I think it's better to sort it 
alphabetically, because everyone who doesn't know 'struct stat' could 
be very puzzled about atime, mtime, ctime, btime.



+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.
Well, actually yes, but the the description on get_stat_birthtime 
says: "Return *ST's birth time, if available; otherwise return a value 
with tv_sec and tv_nsec both equal to -1.". So to be sure I prefer to 
check both.

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.

Ok, please let me know if there are some changes here. Meanwhile I 
will adapt my patch.


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


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

2012-07-16 Thread Hendrik Schwartke

On 13.07.2012 17:14, Eric Blake wrote:

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.
  Since 0.4.1

+timestamps
+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'


+atime,btime,ctime  andmtime
+hold the access, birth, change and modification time of the volume, 
where known.
+The used time format is. since the 
beginning
+of the epoch.  This is a readonly attribute and is ignored when 
creating
+a volume.Since 0.10.0



+
+
+
+
+
+
+[0-9]+\.[0-9]+
+

It might be worth writing the regex to permit eliding the sub-second
resolution, on file systems that only have 1 second resolution.  Given
Well, the problem here is that stat-time doesn't offer a way to 
determine if sub-second resolution is available. If the system doesn't 
support it then tv_nsec is simply zero. So there is always a sub-second 
part in the timestamp and such an regex could be slightly misleading. I 
will change it anyway and add a comment to the schema.

that we are repeating this  four times, it might be worth defining
it, for a shorter diff:

   
 
   

...

   
 [0-9]+(\.[0-9]+)?
   



+++ b/src/conf/storage_conf.c
@@ -1277,6 +1277,24 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr 
options,

  virBufferAddLit(buf,"\n");

+virBufferAddLit(buf, "\n");
+virBufferAsprintf(buf, "%llu.%ld\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)

That's never the case. See above.

 return;
 virBufferAsprintf(buf, "<%s>%llu", name,
   (unsigned long long) ts->tv_sec);
 if (ts->tv_nsec)
 virBufferAsprintf(buf, ".%ld", tv->tv_nsec);
 virBufferAsprintf(buf, "\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).
Well I thought about that and I think it's better to sort it 
alphabetically, because everyone who doesn't know 'struct stat' could be 
very puzzled about atime, mtime, ctime, btime.



+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.
Well, actually yes, but the the description on get_stat_birthtime says: 
"Return *ST's birth time, if available; otherwise return a value with 
tv_sec and tv_nsec both equal to -1.". So to be sure I prefer to check both.

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.

Ok, please let me know if there are some changes here. Meanwhile I will 
adapt my patch.


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


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

2012-07-13 Thread Eric Blake
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.
>  Since 0.4.1
>
> +  timestamps
> +  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'

> +atime, btime, ctime and 
> mtime
> +hold the access, birth, change and modification time of the volume, 
> where known.
> +The used time format is . since 
> the beginning
> +of the epoch.  This is a readonly attribute and is ignored when 
> creating
> +a volume. Since 0.10.0


> +  
> +
> +  
> +
> +  
> +
> +  [0-9]+\.[0-9]+
> +

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  four times, it might be worth defining
it, for a shorter diff:

  

  

...

  
[0-9]+(\.[0-9]+)?
  


> +++ b/src/conf/storage_conf.c
> @@ -1277,6 +1277,24 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr 
> options,
>  
>  virBufferAddLit(buf,"\n");
>  
> +virBufferAddLit(buf, "\n");
> +virBufferAsprintf(buf, "  %llu.%ld\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, "\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   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Added timestamps to storage volumes

2012-07-13 Thread Hendrik Schwartke
!!! 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
 '
 
 # Additional xgettext options to use.  Use "\\\newline" to break lines.
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index d0e4319..ec7ec90 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -141,6 +141,11 @@
 0744
 
   
+  
+1341933637.27319099
+1341930622.47245868
+1341930622.47245868
+  
   
 ...
   
@@ -172,6 +177,14 @@
 contains the MAC (eg SELinux) label string.
 Since 0.4.1
   
+  timestamps
+  Provides timing information about the volume. The four sub elements
+atime, btime, ctime and 
mtime
+hold the access, birth, change and modification time of the volume, 
where known.
+The used time format is . since the 
beginning
+of the epoch.  This is a readonly attribute and is ignored when 
creating
+a volume. Since 0.10.0
+  
   encryption
   If present, specifies how the volume is encrypted.  See
 the Storage Encryption page
diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng
index 7a74331..a20c7a5 100644
--- a/docs/schemas/storagevol.rng
+++ b/docs/schemas/storagevol.rng
@@ -63,6 +63,41 @@
 
   
 
+  
+
+  
+
+  
+
+  [0-9]+\.[0-9]+
+
+  
+
+
+  
+
+  [0-9]+\.[0-9]+
+
+  
+
+
+  
+
+  [0-9]+\.[0-9]+
+
+  
+
+
+  
+
+  [0-9]+\.[0-9]+
+
+  
+
+  
+
+  
+
   
 
   
@@ -72,6 +107,7 @@
   
   
   
+  
   
 
   
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 36a3bb9..b1ec8da 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1277,6 +1277,24 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr 
options,
 
 virBufferAddLit(buf,"\n");
 
+virBufferAddLit(buf, "\n");
+virBufferAsprintf(buf, "  %llu.%ld\n",
+  (unsigned long long) def->timestamps.atime.tv_sec,
+  def->timestamps.atime.tv_nsec);
+if(def->timestamps.btime.tv_sec != -1 ||
+   def->timestamps.btime.tv_nsec != -1) {
+virBufferAsprintf(buf, "  %llu.%ld\n",
+  (unsigned long long) def->timestamps.btime.tv_sec,
+  def->timestamps.btime.tv_nsec);
+}
+virBufferAsprintf(buf, "  %llu.%ld\n",
+  (unsigned long long) def->timestamps.ctime.tv_sec,
+  def->timestamps.ctime.tv_nsec);
+virBufferAsprintf(buf, "  %llu.%ld\n",
+  (unsigned long long) def->timestamps.mtime.tv_sec,
+  def->timestamps.mtime.tv_nsec);
+virBufferAddLit(buf, "\n");
+
 if (def->encryption) {
 virBufferAdjustIndent(buf, 4);
 if (virStorageEncryptionFormat(buf, def->encryption) < 0)
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 5733b57..706e874 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -46,6 +46,18 @@ struct _virStoragePerms {
 
 /* Storage volumes */
 
+typedef struct _virStorageTimestamps virStorageTimestamps;
+typedef virStorageTimestamps *virStorageTimestampsPtr;
+struct _virStorageTimestamps {
+struct timespec atime;
+/* if btime.tv_sec == -1 && btime.tv_nsec == -1 than
+ * birth time is unknown
+ */
+struct timespec btime;
+struct timespec ctime;
+struct timespec mtime;
+};
+
 
 /*
  * How the volume's data is stored on underlying
@@ -77,6 +89,7 @@ struct _virStorageVolTarget {
 char *path;
 int format;
 virStoragePerms perms;
+virStorageTimestamps timestamps;
 int type; /* only used by disk backend for partition type */
 /* Currently used 

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

2012-07-11 Thread Hendrik Schwartke

On 11.07.2012 15:10, Eric Blake wrote:

On 07/11/2012 07:09 AM, Eric Blake wrote:

On 07/11/2012 01:12 AM, Hendrik Schwartke wrote:


+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?

It means 3 of the 4 copyright holders have agreed, with less than
24-hour turnaround time; the 4th will likely respond sometime this week:
https://lists.gnu.org/archive/html/bug-gnulib/2012-07/msg00141.html


Correction - there is only one copyright holder - the FSF.  But 3 of the
4 original contributors have agreed to a relicense, and if all original
contributors agree, then we don't have to bother the FSF with a much
more formal and lengthy process of convincing the FSF to relicense.


Oh, cool. I didn't thought that this could be done in such a short term.
So I will update my patch using stat-time this week so that it is ready 
if the license has changed.

(I will also add the birth time.)

Thanks
Hendrik

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


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

2012-07-11 Thread Eric Blake
On 07/11/2012 07:09 AM, Eric Blake wrote:
> On 07/11/2012 01:12 AM, Hendrik Schwartke wrote:
> 
 +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?
> 
> It means 3 of the 4 copyright holders have agreed, with less than
> 24-hour turnaround time; the 4th will likely respond sometime this week:
> https://lists.gnu.org/archive/html/bug-gnulib/2012-07/msg00141.html
> 

Correction - there is only one copyright holder - the FSF.  But 3 of the
4 original contributors have agreed to a relicense, and if all original
contributors agree, then we don't have to bother the FSF with a much
more formal and lengthy process of convincing the FSF to relicense.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-07-11 Thread Eric Blake
On 07/11/2012 01:12 AM, Hendrik Schwartke wrote:

>>> +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?

It means 3 of the 4 copyright holders have agreed, with less than
24-hour turnaround time; the 4th will likely respond sometime this week:
https://lists.gnu.org/archive/html/bug-gnulib/2012-07/msg00141.html

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

Being a security patch that affects multiple packages, I'm sure it won't
be too long in the works.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-07-11 Thread Hendrik Schwartke

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(+)

+timestamps
+Provides timing information about the volume. The three sub elements
+atime,mtime  andctime  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,"\n");

+virBufferAddLit(buf, "\n");
+virBufferAsprintf(buf, "%llu.%lu\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 */
}



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


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

2012-07-10 Thread Eric Blake
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(+)

>
> +  timestamps
> +  Provides timing information about the volume. The three sub 
> elements
> +atime, mtime and ctime 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,"\n");
>  
> +virBufferAddLit(buf, "\n");
> +virBufferAsprintf(buf, "  %llu.%lu\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   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Added timestamps to storage volumes

2012-07-10 Thread Hendrik Schwartke
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(+)

diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index d0e4319..c4d6d85 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -141,6 +141,11 @@
 0744
 
   
+  
+1341933637.27319099
+1341930622.47245868
+1341930622.47245868
+  
   
 ...
   
@@ -172,6 +177,14 @@
 contains the MAC (eg SELinux) label string.
 Since 0.4.1
   
+  timestamps
+  Provides timing information about the volume. The three sub elements
+atime, mtime and ctime hold the
+access, modification and respectively the change time of the volume. 
The
+used time format is . since the 
beginning
+of the epoch.  This is a readonly attribute and is ignored when 
creating
+a volume. Since 0.10.0
+  
   encryption
   If present, specifies how the volume is encrypted.  See
 the Storage Encryption page
diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng
index 7a74331..dafe918 100644
--- a/docs/schemas/storagevol.rng
+++ b/docs/schemas/storagevol.rng
@@ -63,6 +63,28 @@
 
   
 
+  
+
+  
+
+  
+[0-9]+\.[0-9]+
+  
+
+
+  
+[0-9]+\.[0-9]+
+  
+
+
+  
+[0-9]+\.[0-9]+
+  
+
+  
+
+  
+
   
 
   
@@ -72,6 +94,7 @@
   
   
   
+  
   
 
   
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index ab8df9e..435ad00 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1272,6 +1272,18 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr 
options,
 
 virBufferAddLit(buf,"\n");
 
+virBufferAddLit(buf, "\n");
+virBufferAsprintf(buf, "  %llu.%lu\n",
+  (unsigned long long) def->timestamps.atime.tv_sec,
+  def->timestamps.atime.tv_nsec);
+virBufferAsprintf(buf, "  %llu.%lu\n",
+  (unsigned long long) def->timestamps.mtime.tv_sec,
+  def->timestamps.mtime.tv_nsec);
+virBufferAsprintf(buf, "  %llu.%lu\n",
+  (unsigned long long) def->timestamps.ctime.tv_sec,
+  def->timestamps.ctime.tv_nsec);
+virBufferAddLit(buf, "\n");
+
 if (def->encryption) {
 virBufferAdjustIndent(buf, 4);
 if (virStorageEncryptionFormat(buf, def->encryption) < 0)
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 5733b57..977b136 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -46,6 +46,14 @@ struct _virStoragePerms {
 
 /* Storage volumes */
 
+typedef struct _virStorageTimestamps virStorageTimestamps;
+typedef virStorageTimestamps *virStorageTimestampsPtr;
+struct _virStorageTimestamps {
+struct timespec atime;
+struct timespec mtime;
+struct timespec ctime;
+};
+
 
 /*
  * How the volume's data is stored on underlying
@@ -77,6 +85,7 @@ struct _virStorageVolTarget {
 char *path;
 int format;
 virStoragePerms perms;
+virStorageTimestamps timestamps;
 int type; /* only used by disk backend for partition type */
 /* Currently used only in virStorageVolDef.target, not in .backingstore. */
 virStorageEncryptionPtr encryption;
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index e2e9b51..ce4d808 100644
--- a/src/storage/storage_backend.c
+++ 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,
+*const mtime=&target->timestamps.mtime,
+*const catime=&target->timestamps.ctime;
 #if HAVE_SELINUX
 security_context_t filecon = NULL;
 #endif
@@ -1209,6 +1212,23 @@ 
virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target,
 target->perms.uid = sb.st_uid;
 target->perms.gid = sb.st_gid;
 
+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;
+