On Mon, Oct 17, 2011 at 11:47 PM, Stefan Hajnoczi <stefa...@linux.vnet.ibm.com> wrote: > 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; > + > + 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--; > +} I don't understand what the real intention for the above functions is. IMHO, why can we not drop them?
> + > /* > * 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); > + return ret; > } > > int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num, > @@ -1288,6 +1364,7 @@ static int coroutine_fn > bdrv_co_do_writev(BlockDriverState *bs, > int64_t sector_num, int nb_sectors, QEMUIOVector *qiov) > { > BlockDriver *drv = bs->drv; > + BdrvTrackedRequest *req; > int ret; > > if (!bs->drv) { > @@ -1300,6 +1377,8 @@ static int coroutine_fn > bdrv_co_do_writev(BlockDriverState *bs, > return -EIO; > } > > + req = tracked_request_add(bs, sector_num, nb_sectors, true); > + > ret = drv->bdrv_co_writev(bs, sector_num, nb_sectors, qiov); > > if (bs->dirty_bitmap) { > @@ -1310,6 +1389,8 @@ static int coroutine_fn > bdrv_co_do_writev(BlockDriverState *bs, > bs->wr_highest_sector = sector_num + nb_sectors - 1; > } > > + tracked_request_remove(req); > + > return ret; > } > > diff --git a/block_int.h b/block_int.h > index f2f4f2d..87ce8b4 100644 > --- a/block_int.h > +++ b/block_int.h > @@ -43,6 +43,8 @@ > #define BLOCK_OPT_PREALLOC "preallocation" > #define BLOCK_OPT_SUBFMT "subformat" > > +typedef struct BdrvTrackedRequest BdrvTrackedRequest; > + > typedef struct AIOPool { > void (*cancel)(BlockDriverAIOCB *acb); > int aiocb_size; > @@ -206,6 +208,9 @@ struct BlockDriverState { > int in_use; /* users other than guest access, eg. block migration */ > QTAILQ_ENTRY(BlockDriverState) list; > void *private; > + > + int request_tracking; /* reference count, 0 means off */ > + QLIST_HEAD(, BdrvTrackedRequest) tracked_requests; > }; > > struct BlockDriverAIOCB { > @@ -216,6 +221,9 @@ struct BlockDriverAIOCB { > BlockDriverAIOCB *next; > }; > > +void bdrv_start_request_tracking(BlockDriverState *bs); > +void bdrv_stop_request_tracking(BlockDriverState *bs); > + > void get_tmp_filename(char *filename, int size); > > void *qemu_aio_get(AIOPool *pool, BlockDriverState *bs, > -- > 1.7.6.3 > > > -- Regards, Zhi Yong Wu