Re: [Qemu-devel] [PATCH V4 1/2] qapi: Add SnapshotInfo and ImageInfo.

[Benoît Canet]

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