δΊ 2013-1-5 0:22, Stefan Hajnoczi ει:
On Mon, Dec 17, 2012 at 02:25:08PM +0800, Wenchao Xia wrote:@@ -1478,16 +1497,39 @@ # # @device: the name of the device to generate the snapshot from. # -# @snapshot-file: the target of the new image. A new file will be created. +# @snapshot-file: the target name of the snapshot. In external case, it is +# the new file's name, A new file will be created. In internal +# case, it is the internal snapshot record's name and if it is +# 'blank' name will be generated according to time. # # @format: #optional the format of the snapshot image, default is 'qcow2'. # -# @mode: #optional whether and how QEMU should create a new image, default is -# 'absolute-paths'. +# @mode: #optional whether QEMU should create a new snapshot or use existing +# one, default is 'absolute-paths'. +# +# @type: #optional internal snapshot or external, default is 'external'. +# ## { 'type': 'BlockdevSnapshot', 'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str', - '*mode': 'NewImageMode' } } + '*mode': 'NewImageMode', '*type': 'SnapshotType'} }It feels cleaner to define a new BlockdevInternalSnapshot type instead of overloading the field semantics. The naming doesn't fit for internal snapshots (e.g. "snapshot-file", "absolute-paths", etc). Treat internal snapshots as a new transaction action. That way we can cleanly add internal snapshot specific fields in the future. The documentation will be clearer because users won't have to consider the "if type is 'external', then ..., otherwise ...". Stefan
After struggling about the parameter meaning, I guess this is what make things better. -- Best Regards Wenchao Xia
