[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extenda
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extendable |
Date: |
Thu, 18 Apr 2013 08:35:16 +0200 |
On Thu, Apr 18, 2013 at 5:00 AM, Wenchao Xia <address@hidden> wrote:
> δΊ 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?
No explicit pointer is needed since ExternalSnapshotState embeds
BlkTransactionState. See container_of() or DO_UPCAST().
Stefan
- [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extendable, (continued)
- [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extendable, Wenchao Xia, 2013/04/01
- Re: [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extendable, Kevin Wolf, 2013/04/02
- Re: [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extendable, Wenchao Xia, 2013/04/03
- Re: [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extendable, Paolo Bonzini, 2013/04/03
- Re: [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extendable, Wenchao Xia, 2013/04/03
- Re: [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extendable, Paolo Bonzini, 2013/04/03
- Re: [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extendable, Kevin Wolf, 2013/04/03
- Re: [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extendable, Wenchao Xia, 2013/04/03
Re: [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extendable, Stefan Hajnoczi, 2013/04/17
[Qemu-devel] [PATCH 3/3] block: change rollback sequence in qmp_transaction, Wenchao Xia, 2013/04/01