|
From: | Wenchao Xia |
Subject: | Re: [Qemu-devel] [PATCH V2 07/10] snapshot: qmp use new internal API for external snapshot transaction |
Date: | Mon, 18 Mar 2013 18:00:36 +0800 |
User-agent: | Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/20130307 Thunderbird/17.0.4 |
于 2013-3-14 16:22, Kevin Wolf 写道:
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. KevinMy 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?
This seems to complex, since prepare of all actions are executed in first round, they may interrupt each other later. So I am thinking make it more clear as complete one job one time, which may change the old qmp_transcation() logic a little.
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.
In above way check/prepare can be merged, how do you think of it?
Kevin
-- Best Regards Wenchao Xia
[Prev in Thread] | Current Thread | [Next in Thread] |