On Tue, Aug 9, 2011 at 8:49 PM, Stefan Hajnoczi <stefanha@xxxxxxxxx> wrote: > On Tue, Aug 9, 2011 at 5:17 AM, Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx> wrote: >> +/* The APIs for block request queue on qemu block layer. >> + */ >> + >> +static void qemu_block_queue_cancel(BlockDriverAIOCB *acb) >> +{ >> + qemu_aio_release(acb); >> +} >> + >> +static AIOPool block_queue_pool = { >> + .aiocb_size = sizeof(struct BlockDriverAIOCB), >> + .cancel = qemu_block_queue_cancel, >> +}; > > The lifecycle of block_queue_pool acbs should be like this: > > When a request is queued we need a BlockQueue acb because we have no > real acb yet. So we get an acb from block_queue_pool. > > If the acb is cancelled before being dispatched we need to release the > acb and remove the request from the queue. (This patch does not > remove the request from the queue on cancel.) > > When the acb is dispatched we need to keep track of the real acb (e.g. > from qcow2). The caller will only ever see our acb because there is > no way to give them the pointer to the new acb after dispatch. That > means we need to keep the our acb alive for the entire lifetime of the > request. (This patch currently releases our acb when the request is > dispatched.) > > If the acb is cancelled after being dispatched we need to first cancel > the real acb and then release our acb. > > When the acb is dispatched we need to pass qemu_block_queue_callback > as the cb function to handler. Inside qemu_block_queue_callback we When one block request have been reenqueued multiple times, it will be difficult to store original cb function. > invoke the request cb and then release our acb. This is how our acb > should be freed. > >> +BlockDriverAIOCB *qemu_block_queue_enqueue(BlockQueue *queue, >> + BlockDriverState *bs, >> + BlockRequestHandler *handler, >> + int64_t sector_num, >> + QEMUIOVector *qiov, >> + int nb_sectors, >> + BlockDriverCompletionFunc *cb, >> + void *opaque) >> +{ >> + BlockIORequest *request; >> + BlockDriverAIOCB *acb; >> + >> + request = qemu_malloc(sizeof(BlockIORequest)); >> + request->bs = bs; >> + request->handler = handler; >> + request->sector_num = sector_num; >> + request->qiov = qiov; >> + request->nb_sectors = nb_sectors; >> + request->cb = cb; >> + request->opaque = opaque; >> + >> + QTAILQ_INSERT_TAIL(&queue->requests, request, entry); > > It would be simpler to define BlockQueueAIOCB and using it as our acb > instead of managing an extra BlockIORequest structure. That way you > don't need to worry about extra mallocs and frees. > >> + >> + acb = qemu_aio_get(&block_queue_pool, bs, >> + qemu_block_queue_callback, opaque); >> + >> + request->acb = acb; >> + >> + return acb; >> +} >> + >> +int qemu_block_queue_handler(BlockIORequest *request) > > This function can be static. > > Stefan > -- Regards, Zhi Yong Wu -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html