qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH V4 0/5] block: make qmp_transaction extendable


From: Kevin Wolf
Subject: Re: [Qemu-devel] [PATCH V4 0/5] block: make qmp_transaction extendable
Date: Fri, 3 May 2013 10:46:26 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

Am 02.05.2013 um 04:26 hat Wenchao Xia geschrieben:
>   This serial will package backing chain snapshot code as one case, to make it
> possible adding more operations later.
> 
> v2:
>   Address Kevin's comments:
>   Use the same prototype prepare, commit, rollback model in original code,
> commit should never fail.
> 
> v3:
>   Address Stefan's comments:
>   3/5, 4/5: remove *action parameter since later only BlkTransactionStates* is
> needed.
>   5/5: embbed BlkTransactionStates in ExternalSnapshotStates, *opaque is
> removed, related call back function format change for external snapshot.
>   Address Kevin's comments:
>   removed all indention in commit message.
>   1/5: return void for prepare() function, *errp plays the role as error
> checker.
>   5/5: mark *commit callback must exist, *rollback callback can be NULL. Align
> "callback =" in "const BdrvActionOps external_snapshot_ops" to the same colum.
>   Address Eric's comments:
>   1/5: better commit message.
>   5/5: better commit message and comments in code that only one of rollback()
> or commit() will be called.
> 
> v4:
>   5/5: document clean() callback will always be called if it present, declare
> static for global variable "actions", use array plus .instance_size to remove
> "switch" checking code according to caller input.
> 
> Wenchao Xia (5):
>   1 block: package preparation code in qmp_transaction()
>   2 block: move input parsing code in qmp_transaction()
>   3 block: package committing code in qmp_transaction()
>   4 block: package rollback code in qmp_transaction()
>   5 block: make all steps in qmp_transaction() as callback
> 
>  blockdev.c |  263 ++++++++++++++++++++++++++++++++++++++---------------------
>  1 files changed, 169 insertions(+), 94 deletions(-)

I would have used s/rollback/abort/ for consistency with bdrv_reopen_*(),
but this doesn't make a difference for the correctness, so:

Reviewed-by: Kevin Wolf <address@hidden>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]