Re: [Qemu-devel] [PATCH V2 07/10] snapshot: qmp use new internal API for external snapshot transaction

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 2013-1-9 20:44, Stefan Hajnoczi 写道:
> On Mon, Jan 07, 2013 at 03:28:06PM +0800, Wenchao Xia wrote:
>>    This patch switch to internal common API to take group external
>> snapshots from qmp_transaction interface. qmp layer simply does
>> a translation from user input.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   blockdev.c |  215 ++++++++++++++++++++++++------------------------------------
>>   1 files changed, 87 insertions(+), 128 deletions(-)
>
> An internal API for snapshots is not necessary.  qmp_transaction() is
> already usable both from the monitor and C code.
>
> The QAPI code generator creates structs that can be accessed directly
> from C.  qmp_transaction(), BlockdevAction, and BlockdevActionList *is*
> the snapshot API.  It just doesn't support internal snapshots yet, which
> is what you are trying to add.
>
> To add internal snapshot support, define a BlockdevInternalSnapshot type
> in qapi-schema.json and add internal snapshot support in
> qmp_transaction().
>
> qmp_transaction() was designed with this in mind from the beginning and
> dispatches based on BlockdevAction->kind.
>
> The patch series will become much smaller while still adding internal
> snapshot support.
>
> Stefan
>

   As API, qmp_transaction have following disadvantages:
1) interface is based on string not data type inside qemu, that means
other function calling it result in: bdrv->string->bdrv
2) all capability are forced to be exposed.
3) need structure to record each transaction state, such as
BlkTransactionStates. Extending it is equal to add an internal layer.

   Actually I started up by use qmp_transaction as API, but soon
found that work is almost done around BlkTransactionStates, so
added a layer around it clearly.

-- 
Best Regards

Wenchao Xia