Patchwork [V4,1/2] qapi: Add SnapshotInfo and ImageInfo.

login
register
mail settings
Submitter Benoit Canet
Date Aug. 22, 2012, 12:45 p.m.
Message ID <1345639535-8822-2-git-send-email-benoit@irqsave.net>
Download mbox | patch
Permalink /patch/179311/
State New
Headers show

Comments

Benoit Canet - Aug. 22, 2012, 12:45 p.m.
Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 qapi-schema.json |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)
Eric Blake - Aug. 22, 2012, 2:03 p.m.
On 08/22/2012 06:45 AM, Benoît Canet wrote:
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
>  qapi-schema.json |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index a92adb1..4c4b9b9 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -126,6 +126,68 @@
>              'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
>  
>  ##
> +# @SnapshotInfo
> +#
> +# @id: unique snapshot id
> +#
> +# @name: user choosen name

s/choosen/chosen/

> +#
> +# @vm-state-size: size of the VM state
> +#
> +# @date-sec: UTC date of the snapshot
> +#
> +# @date-nsec: date in nano seconds
> +#
> +# @vm-clock-nsec: VM clock relative to boot in nano seconds

Since we have two fields named *-nsec, it might be worth clarifying that
date-nsec is merely the fractional portion to be combined with date-sec
(always less than 1000000000), while vm-clock-nsec includes seconds if
the drift is that large.

For that matter, should we even be exposing things in this manner?  I
know the internal struct has seconds and nanos separate for date,
because it maps to struct timespec; but why can't we combine them into
one giant number for JSON?  Or are we worried about exceeding int64_t if
we take seconds * 1000000000 when dealing with the timestamp the
snapshot was created, even though we aren't worried about the VM clock
being that far away from boot?

> +#
> +# Since: 1.2

Probably too late for 1.2, so this should be 1.3.

> +##
> +# @ImageInfo:
> +#
> +# Information about a QEMU image file
> +#
> +# @filename: name of the image file
> +#
> +# @format: format of the image file
> +#
> +# @virtual-size: maximum capacity in bytes of the image
> +#
> +# @actual-size: #optional actual size on disk in bytes of the image

That seems backwards - with a raw file, don't you know the actual size,
but have no idea what it can further grow to?

> +#
> +# @dirty-flag: #optional true if image is not cleanly closed
> +#
> +# @cluster-size: #optional size of a cluster in bytes
> +#
> +# @encrypted: #optional true if the image is encrypted
> +#
> +# @backing-filename: #optional name of the backing file
> +#
> +# @full-backing-filename: #optional full path of the backing file
> +#
> +# @backing-filename-format: #optional the format of the backing file
> +#
> +# @snapshots: #optional list of VM snapshots
> +#
> +# Since: 1.2

Again, 1.3.
Benoît Canet - Aug. 22, 2012, 2:32 p.m.
Le Wednesday 22 Aug 2012 à 08:03:14 (-0600), Eric Blake a écrit :
> On 08/22/2012 06:45 AM, Benoît Canet wrote:
> > Signed-off-by: Benoit Canet <benoit@irqsave.net>
> > ---
> >  qapi-schema.json |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 62 insertions(+)
> > 
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index a92adb1..4c4b9b9 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -126,6 +126,68 @@
> >              'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
> >  
> >  ##
> > +# @SnapshotInfo
> > +#
> > +# @id: unique snapshot id
> > +#
> > +# @name: user choosen name
> 
> s/choosen/chosen/
> 
> > +#
> > +# @vm-state-size: size of the VM state
> > +#
> > +# @date-sec: UTC date of the snapshot
> > +#
> > +# @date-nsec: date in nano seconds
> > +#
> > +# @vm-clock-nsec: VM clock relative to boot in nano seconds
> 
> Since we have two fields named *-nsec, it might be worth clarifying that
> date-nsec is merely the fractional portion to be combined with date-sec
> (always less than 1000000000), while vm-clock-nsec includes seconds if
> the drift is that large.
> 
> For that matter, should we even be exposing things in this manner?  I
> know the internal struct has seconds and nanos separate for date,
> because it maps to struct timespec; but why can't we combine them into
> one giant number for JSON?

Wouldn't people working with low level language be annoyed after parsing
this JSON to have to split this combined number in two parts to fit
them back into struct timespec ?

> Or are we worried about exceeding int64_t if
> we take seconds * 1000000000 when dealing with the timestamp the
> snapshot was created, even though we aren't worried about the VM clock
> being that far away from boot?
> 
> > +#
> > +# Since: 1.2
> 
> Probably too late for 1.2, so this should be 1.3.
> 
> > +##
> > +# @ImageInfo:
> > +#
> > +# Information about a QEMU image file
> > +#
> > +# @filename: name of the image file
> > +#
> > +# @format: format of the image file
> > +#
> > +# @virtual-size: maximum capacity in bytes of the image
> > +#
> > +# @actual-size: #optional actual size on disk in bytes of the image
> 
> That seems backwards - with a raw file, don't you know the actual size,
> but have no idea what it can further grow to?
> 
> > +#
> > +# @dirty-flag: #optional true if image is not cleanly closed
> > +#
> > +# @cluster-size: #optional size of a cluster in bytes
> > +#
> > +# @encrypted: #optional true if the image is encrypted
> > +#
> > +# @backing-filename: #optional name of the backing file
> > +#
> > +# @full-backing-filename: #optional full path of the backing file
> > +#
> > +# @backing-filename-format: #optional the format of the backing file
> > +#
> > +# @snapshots: #optional list of VM snapshots
> > +#
> > +# Since: 1.2
> 
> Again, 1.3.
> 
> -- 
> Eric Blake   eblake@redhat.com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
Eric Blake - Aug. 22, 2012, 2:56 p.m.
On 08/22/2012 08:32 AM, Benoît Canet wrote:
>> Since we have two fields named *-nsec, it might be worth clarifying that
>> date-nsec is merely the fractional portion to be combined with date-sec
>> (always less than 1000000000), while vm-clock-nsec includes seconds if
>> the drift is that large.
>>
>> For that matter, should we even be exposing things in this manner?  I
>> know the internal struct has seconds and nanos separate for date,
>> because it maps to struct timespec; but why can't we combine them into
>> one giant number for JSON?
> 
> Wouldn't people working with low level language be annoyed after parsing
> this JSON to have to split this combined number in two parts to fit
> them back into struct timespec ?

Perhaps, in which case, why don't we present vm-clock-nsec via two
fields of seconds and fraction, for the same reasoning?  My point is
that we have two different bike shed colors showing in this one API, but
I would prefer we be consistent and pick just one (as to _which_ color,
I can be persuaded either way).
Benoît Canet - Aug. 22, 2012, 3:51 p.m.
Le Wednesday 22 Aug 2012 à 08:56:06 (-0600), Eric Blake a écrit :
> On 08/22/2012 08:32 AM, Benoît Canet wrote:
> >> Since we have two fields named *-nsec, it might be worth clarifying that
> >> date-nsec is merely the fractional portion to be combined with date-sec
> >> (always less than 1000000000), while vm-clock-nsec includes seconds if
> >> the drift is that large.
> >>
> >> For that matter, should we even be exposing things in this manner?  I
> >> know the internal struct has seconds and nanos separate for date,
> >> because it maps to struct timespec; but why can't we combine them into
> >> one giant number for JSON?
> > 
> > Wouldn't people working with low level language be annoyed after parsing
> > this JSON to have to split this combined number in two parts to fit
> > them back into struct timespec ?
> 
> Perhaps, in which case, why don't we present vm-clock-nsec via two
> fields of seconds and fraction, for the same reasoning?
I'll do that.

> My point is
> that we have two different bike shed colors showing in this one API, but
> I would prefer we be consistent and pick just one (as to _which_ color,
> I can be persuaded either way).
> 
> -- 
> Eric Blake   eblake@redhat.com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
Benoît Canet - Aug. 23, 2012, 7:34 a.m.
> > +##
> > +# @ImageInfo:
> > +#
> > +# Information about a QEMU image file
> > +#
> > +# @filename: name of the image file
> > +#
> > +# @format: format of the image file
> > +#
> > +# @virtual-size: maximum capacity in bytes of the image
> > +#
> > +# @actual-size: #optional actual size on disk in bytes of the image
> 
> That seems backwards - with a raw file, don't you know the actual size,
> but have no idea what it can further grow to?
Try the current qemy-img info on a raw file you will be surprised of the results.
This field also can have an "unavailable" value.

example result with unpatched qemu-img:

benoit@Laure:~/qemu$ qemu-img info truc1.raw 
image: truc1.raw
file format: raw
virtual size: 1.0G (1073741824 bytes)
disk size: 776M

disk size is actual-size.

Benoît
Benoît Canet - Aug. 23, 2012, 7:38 a.m.
> That seems backwards - with a raw file, don't you know the actual size,
> but have no idea what it can further grow to?
In raw it commes from fstat() with the field st.st_blocks.

from man:
blkcnt_t  st_blocks;  /* number of 512B blocks allocated */

Benoît

Patch

diff --git a/qapi-schema.json b/qapi-schema.json
index a92adb1..4c4b9b9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -126,6 +126,68 @@ 
             'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
 
 ##
+# @SnapshotInfo
+#
+# @id: unique snapshot id
+#
+# @name: user choosen name
+#
+# @vm-state-size: size of the VM state
+#
+# @date-sec: UTC date of the snapshot
+#
+# @date-nsec: date in nano seconds
+#
+# @vm-clock-nsec: VM clock relative to boot in nano seconds
+#
+# Since: 1.2
+#
+##
+
+{ 'type': 'SnapshotInfo',
+  'data': { 'id': 'str', 'name': 'str', 'vm-state-size': 'int',
+            'date-sec': 'int', 'date-nsec': 'int',
+            'vm-clock-nsec': 'int' } }
+
+##
+# @ImageInfo:
+#
+# Information about a QEMU image file
+#
+# @filename: name of the image file
+#
+# @format: format of the image file
+#
+# @virtual-size: maximum capacity in bytes of the image
+#
+# @actual-size: #optional actual size on disk in bytes of the image
+#
+# @dirty-flag: #optional true if image is not cleanly closed
+#
+# @cluster-size: #optional size of a cluster in bytes
+#
+# @encrypted: #optional true if the image is encrypted
+#
+# @backing-filename: #optional name of the backing file
+#
+# @full-backing-filename: #optional full path of the backing file
+#
+# @backing-filename-format: #optional the format of the backing file
+#
+# @snapshots: #optional list of VM snapshots
+#
+# Since: 1.2
+#
+##
+
+{ 'type': 'ImageInfo',
+  'data': {'filename': 'str', 'format': 'str', '*dirty-flag': 'bool',
+           '*actual-size': 'int', 'virtual-size': 'int',
+           '*cluster-size': 'int', '*encrypted': 'bool',
+           '*backing-filename': 'str', '*full-backing-filename': 'str',
+           '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] } }
+
+##
 # @StatusInfo:
 #
 # Information about VCPU run state