On Wed, Nov 2, 2011 at 4:30 PM, Kevin Wolf <kw...@redhat.com> wrote:
> Am 17.10.2011 17:47, schrieb Stefan Hajnoczi:
>> The block layer does not know about pending requests. This information
>> is necessary for copy-on-read since overlapping requests must be
>> serialized to prevent races that corrupt the image.
>>
>> Add a simple mechanism to enable/disable request tracking. By default
>> request tracking is disabled.
>>
>> The BlockDriverState gets a new tracked_request list field which
>> contains all pending requests. Each request is a BdrvTrackedRequest
>> record with sector_num, nb_sectors, and is_write fields.
>>
>> Signed-off-by: Stefan Hajnoczi <stefa...@linux.vnet.ibm.com>
>> ---
>> block.c | 83
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> block_int.h | 8 +++++
>> 2 files changed, 90 insertions(+), 1 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 9873b57..2d2c62a 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -978,6 +978,77 @@ void bdrv_commit_all(void)
>> }
>> }
>>
>> +struct BdrvTrackedRequest {
>> + BlockDriverState *bs;
>> + int64_t sector_num;
>> + int nb_sectors;
>> + bool is_write;
>> + QLIST_ENTRY(BdrvTrackedRequest) list;
>> +};
>> +
>> +/**
>> + * Remove an active request from the tracked requests list
>> + *
>> + * If req is NULL, no operation is performed.
>> + *
>> + * This function should be called when a tracked request is completing.
>> + */
>> +static void tracked_request_remove(BdrvTrackedRequest *req)
>> +{
>> + if (req) {
>> + QLIST_REMOVE(req, list);
>> + g_free(req);
>> + }
>> +}
>> +
>> +/**
>> + * Add an active request to the tracked requests list
>> + *
>> + * Return NULL if request tracking is disabled, non-NULL otherwise.
>> + */
>> +static BdrvTrackedRequest *tracked_request_add(BlockDriverState *bs,
>> + int64_t sector_num,
>> + int nb_sectors, bool
>> is_write)
>> +{
>> + BdrvTrackedRequest *req;
>> +
>> + if (!bs->request_tracking) {
>> + return NULL;
>> + }
>> +
>> + req = g_malloc(sizeof(*req));
>> + req->bs = bs;
>> + req->sector_num = sector_num;
>> + req->nb_sectors = nb_sectors;
>> + req->is_write = is_write;
>
> How about using g_malloc0 or a compound literal assignment for
> initialisation, so that we won't get surprises when the struct is extended?
Sure.
>> +
>> + QLIST_INSERT_HEAD(&bs->tracked_requests, req, list);
>> +
>> + return req;
>> +}
>> +
>> +/**
>> + * Enable tracking of incoming requests
>> + *
>> + * Request tracking can be safely used by multiple users at the same time,
>> + * there is an internal reference count to match start and stop calls.
>> + */
>> +void bdrv_start_request_tracking(BlockDriverState *bs)
>> +{
>> + bs->request_tracking++;
>> +}
>> +
>> +/**
>> + * Disable tracking of incoming requests
>> + *
>> + * Note that in-flight requests are still tracked, this function only stops
>> + * tracking incoming requests.
>> + */
>> +void bdrv_stop_request_tracking(BlockDriverState *bs)
>> +{
>> + bs->request_tracking--;
>> +}
>> +
>> /*
>> * Return values:
>> * 0 - success
>> @@ -1262,6 +1333,8 @@ static int coroutine_fn
>> bdrv_co_do_readv(BlockDriverState *bs,
>> int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
>> {
>> BlockDriver *drv = bs->drv;
>> + BdrvTrackedRequest *req;
>> + int ret;
>>
>> if (!drv) {
>> return -ENOMEDIUM;
>> @@ -1270,7 +1343,10 @@ static int coroutine_fn
>> bdrv_co_do_readv(BlockDriverState *bs,
>> return -EIO;
>> }
>>
>> - return drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
>> + req = tracked_request_add(bs, sector_num, nb_sectors, false);
>> + ret = drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
>> + tracked_request_remove(req);
>
> Hm... Do we actually need to malloc the BdrvTrackedRequest? If all users
> are like this (and at least those in this patch are), we can just keep
> it on the coroutine stack.
Okay.
Stefan
