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

[Stefan Hajnoczi <stefanha@gmail.com>]

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.

> +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.