δΊ 2013-4-17 22:42, Stefan Hajnoczi ει:
On Mon, Apr 01, 2013 at 06:01:30PM +0800, Wenchao Xia wrote:
/* New and old BlockDriverState structs for group snapshots */
-typedef struct BlkTransactionStates {
+typedef struct BdrvActionOps {
+ int (*commit)(BlockdevAction *action, void **p_opaque, Error **errp);
+ void (*rollback)(BlockdevAction *action, void *opaque);
+ void (*clean)(BlockdevAction *action, void *opaque);
Please document these functions.
OK.
+const BdrvActionOps external_snapshot_ops = {
+ .commit = external_snapshot_commit,
+ .rollback = external_snapshot_rollback,
+ .clean = external_snapshot_clean,
+};
+
+typedef struct BlkTransactionStates {
+ BlockdevAction *action;
+ void *opaque;
+ const BdrvActionOps *ops;
QSIMPLEQ_ENTRY(BlkTransactionStates) entry;
} BlkTransactionStates;
The relationship between BlkTransactionStates and ExternalSnapshotState
can be simplified:
typedef struct {
int (*commit)(BlkTransactionState *state, Error **errp);
void (*rollback)(BlkTransactionState *state);
void (*clean)(BlkTransactionState *state);
size_t instance_size;
} BdrvActionOps;
typedef struct BlkTransactionState {
BlockDevAction *action;
const BdrvActionOps *ops;
QSIMPLEQ_ENTRY(BlkTransactionState) entry;
} BlkTransactionState;
typedef struct {
BlkTransactionState common;
BlockDriverState *old_bs;
BlockDriverState *new_bs;
} ExternalSnapshotState;
const BdrvActionOps external_snapshot_ops = {
.commit = external_snapshot_commit,
.rollback = external_snapshot_rollback,
.clean = external_snapshot_clean,
.instance_size = sizeof(ExternalSnapshotState);
};
This eliminates the opaque pointer and g_free(state) can be handled by
qmp_transaction(). This way action types no longer need to free opaque.
Where should be ExternalSnapshotState* placed, insert it into
BlkTransactionState, or BdrvActionOps?
--
Best Regards
Wenchao Xia