Re: [Qemu-devel] [PATCH V6 11/14] qmp: add interface query-snapshots

[Eric Blake]

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", ...
+                     }
+                  ]
+               }
+            }
+         }
+      ]
+   }


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

signature.asc
Description: OpenPGP digital signature