Re: [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extendable

[Wenchao Xia]

于 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