Re: [Qemu-devel] [PATCH v3 1/8] block: add bdrv_add_before_write_cb()

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

Reviewed the code, except Paolo's comments, function seems fine.

> Il 15/05/2013 16:34, Stefan Hajnoczi ha scritto:
>> The bdrv_add_before_write_cb() function installs a callback that is
>> invoked before a write request is processed.  This will be used to
>> implement copy-on-write point-in-time snapshots where we need to copy
>> out old data before overwriting it.
>
> Perhaps a notifier list that receives the BdrvTrackedRequest?  (BTW we
> should probably remove all the notifier_remove wrappers, they're useless).
>
> The BdrvTrackedRequest pointer would also act as a unique id of the request.
>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>>   block.c                   | 37 +++++++++++++++++++++++++++++++++++++
>>   include/block/block_int.h | 32 ++++++++++++++++++++++++++++++++
>>   2 files changed, 69 insertions(+)
>>
>> diff --git a/block.c b/block.c
>> index 3f87489..0fd7167 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -308,6 +308,7 @@ BlockDriverState *bdrv_new(const char *device_name)
>>       }
>>       bdrv_iostatus_disable(bs);
>>       notifier_list_init(&bs->close_notifiers);
>> +    QTAILQ_INIT(&bs->before_write_cbs);
>>
>>       return bs;
>>   }
>> @@ -1383,6 +1384,8 @@ void bdrv_close(BlockDriverState *bs)
>>           bs->growable = 0;
>>           QDECREF(bs->options);
>>           bs->options = NULL;
>> +        assert(QTAILQ_EMPTY(&bs->before_write_cbs));
>> +        QTAILQ_INIT(&bs->before_write_cbs);
>
> INIT not needed if you assert before.
>
> Paolo
>


-- 
Best Regards

Wenchao Xia