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

[Kevin Wolf <kwolf@redhat.com>]

Am 14.03.2013 um 06:08 hat Wenchao Xia geschrieben:
> 于 2013-3-13 18:18, Kevin Wolf 写道:
> >Am 12.03.2013 um 09:30 hat Wenchao Xia geschrieben:
> >>   I redesigned the structure, Following is the fake code:
> >>
> >>typedef struct BdrvActionOps {
> >>     /* check the request's validation, allocate p_opaque if needed */
> >>     int (*check)(BlockdevAction *action, void **p_opaque, Error **errp);
> >>     /* take the action */
> >>     int (*submit)(BlockdevAction *action, void *opaque, Error **errp);
> >>     /* update emulator */
> >>     int (*commit)(BlockdevAction *action, void *opaque, Error **errp);
> >>     /* cancel the action */
> >>     int (*rollback)(BlockdevAction *action, void *opaque, Error **errp);
> >>} BdrvActionOps;
> >
> >Why do you need the split of prepare into check/submit?
> >
> >If you have prepare/commit/abort, everybody will recognise this as the
> >standard transaction pattern because this is just how it's done.
> >Deviating from it needs a good justification in my opinion.
> >
> >Kevin
> >
> 
>   My thought is rejecting the request in *check if parameter invalid
> before take any action, while submit do the real action, to reduce
> the chance to of rolling back when some request not valid in the batch.

Okay, so it's not strictly needed, but an optimisation of the error
case?

Does it work well when the transaction includes an operation that
depends on the previous one, like create a snapshot and then do
something with this snapshot?

Anyway, even if we think it works and is worth the effort to optimise
such error cases, please use names that are consistent with the
transactions used for reopening: (check/)prepare/commit/abort.

Kevin