On 02/20/2013 03:57 PM, Eric Blake wrote:
On 02/18/2013 03:46 PM, Wenchao Xia wrote:
Hi, Eric
About the interface,there is actually requirement to know internal
snapshots in an image of a backing file, so I think the API should be
improved as:
# @query-snapshots:
#
# Get a list of internal snapshots for whole virtual machine or a single
# block device. Note that in first case, only valid internal snapshot
# will be returned, inconsistent ones will be ignored.
#
# @device: #optional the name of the device to get snapshot info from. #
If not specified, only valid snapshots for whole vm would be
# returned.
# @image: #optional the image's name in the backing chain, only valid
# when device is specified. If it is not specified, the
# internal snapshots on the top of the chain will be shown.
# Otherwise qemu will try search the image on the chain on
# that device.
Why not just always show all information for all images in the chain?
Is it an efficiency issue, where allowing the user to limit information
up front will result in a more responsive command? If that is not a
concern, then making the command simpler by having just a @device
argument, and no @image argument, seems like the better way to go.
But I definitely agree that if you have:
base.qcow2 <- top.qcow2
and both base.qcow2 and top.qcow2 each contain internal snapshots, that
there should be a way to get at all of that information, and not be
limited to just the information on the internal snapshots in top.qcow2.
Thinking more about it, a consistent snapshot is one that we can revert
to right now, without having to do any block device munging. So if
@device is not supplied, we are limited to JUST snapshots at the top of
any image chain (any internal snapshots embedded in a backing file can
only be reached for purposes of a loadvm if we also rearrange the block
device to point to the backing file). But the moment you are interested
in looking at the snapshot information stored in a single device,
without regards to the rest of the system, then knowing everything about
the chain makes sense. But looking at your example:
+-> { "execute": "query-snapshots" }
+<- {
+ "return":[
+ {
+ "id": "1",
+ "name": "snapshot1",
+ "vm-state-size": 0,
+ "date-sec": 10000200,
+ "date-nsec": 12,
+ "vm-clock-sec": 206,
+ "vm-clock-nsec": 30
+ },
+ {
+ "id": "2",
+ "name": "snapshot2",
+ "vm-state-size": 34000000,
+ "date-sec": 13000200,
+ "date-nsec": 32,
+ "vm-clock-sec": 406,
+ "vm-clock-nsec": 31
+ }
+ ]
+ }
that does not give us a way to see which image in a backing chain owns
which internal snapshot names.
I think the best plan of attack is to not try and confuse the two tasks.
Looking at earlier in your series, I think that 'query-images' is the
best place to return information about the entire backing chain AND all
internal snapshots at each point in the backing chain.
That is, leave 'query-snapshots' for JUST obtaining a list of snapshots
that 'loadvm' would work on (only consistent snapshots, present in only
the top image, and no optional @device parameter), and make
'query-images' be the work-horse for determining all details about a
backing chain, something like:
+-> { "execute": "query-images" }
+<- {
+ "return":[
+ {
+ "device":"ide0-hd0",
+ "image":{
+ "filename":"disks/test0.img",
+ "format":"qcow2",
+ "virtual-size":1024000
+ }
+ },
+ {
+ "device":"ide0-hd1",
+ "image":{
+ "filename":"disks/test1.img",
+ "format":"qcow2",
+ "virtual-size":2048000,
+ "snapshots":[
+ {
+ "id": "1",
+ "name": "snapshot1",
+ "vm-state-size": 0,
+ "date-sec": 10000200,
+ "date-nsec": 12,
+ "vm-clock-sec": 206,
+ "vm-clock-nsec": 30
+ }
+ ],
+ "backing":{
+ "filename":"disks/test1base.img",
+ "format":"qcow2",
+ "virtual-size":2048000,
+ "snapshots":[
+ {
+ "id":"2", ...
+ }
+ ]
+ }
+ }
+ }
+ ]
+ }