From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:41466) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qr1kU-0000ia-Tl for qemu-devel@nongnu.org; Wed, 10 Aug 2011 01:54:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qr1kQ-0005hr-6S for qemu-devel@nongnu.org; Wed, 10 Aug 2011 01:54:38 -0400 Received: from mail-yi0-f45.google.com ([209.85.218.45]:50593) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qr1kQ-0005hn-1A for qemu-devel@nongnu.org; Wed, 10 Aug 2011 01:54:34 -0400 Received: by yih10 with SMTP id 10so582436yih.4 for ; Tue, 09 Aug 2011 22:54:33 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1312863472-6901-1-git-send-email-wuzhy@linux.vnet.ibm.com> <1312863472-6901-3-git-send-email-wuzhy@linux.vnet.ibm.com> Date: Wed, 10 Aug 2011 13:54:33 +0800 Message-ID: From: Zhi Yong Wu Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v5 2/4] block: add the block queue support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: kwolf@redhat.com, pair@us.ibm.com, stefanha@linux.vnet.ibm.com, kvm@vger.kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org, ryanh@us.ibm.com, Zhi Yong Wu , luowenj@cn.ibm.com On Tue, Aug 9, 2011 at 8:49 PM, Stefan Hajnoczi wrote: > On Tue, Aug 9, 2011 at 5:17 AM, Zhi Yong Wu wr= ote: >> +/* The APIs for block request queue on qemu block layer. >> + */ >> + >> +static void qemu_block_queue_cancel(BlockDriverAIOCB *acb) >> +{ >> + =A0 =A0qemu_aio_release(acb); >> +} >> + >> +static AIOPool block_queue_pool =3D { >> + =A0 =A0.aiocb_size =A0 =A0 =A0 =A0 =3D sizeof(struct BlockDriverAIOCB)= , >> + =A0 =A0.cancel =A0 =A0 =A0 =A0 =A0 =A0 =3D 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. =A0So 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. =A0(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). =A0The caller will only ever see our acb because there is > no way to give them the pointer to the new acb after dispatch. =A0That > means we need to keep the our acb alive for the entire lifetime of the > request. =A0(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. =A0Inside qemu_block_queue_callback we > invoke the request cb and then release our acb. =A0This is how our acb > should be freed. To the point, very good. > >> +BlockDriverAIOCB *qemu_block_queue_enqueue(BlockQueue *queue, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0BlockDriverState *bs, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0BlockRequestHandler *ha= ndler, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int64_t sector_num, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0QEMUIOVector *qiov, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int nb_sectors, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0BlockDriverCompletionFu= nc *cb, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0void *opaque) >> +{ >> + =A0 =A0BlockIORequest *request; >> + =A0 =A0BlockDriverAIOCB *acb; >> + >> + =A0 =A0request =3D qemu_malloc(sizeof(BlockIORequest)); >> + =A0 =A0request->bs =3D bs; >> + =A0 =A0request->handler =3D handler; >> + =A0 =A0request->sector_num =3D sector_num; >> + =A0 =A0request->qiov =3D qiov; >> + =A0 =A0request->nb_sectors =3D nb_sectors; >> + =A0 =A0request->cb =3D cb; >> + =A0 =A0request->opaque =3D opaque; >> + >> + =A0 =A0QTAILQ_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. =A0That way you > don't need to worry about extra mallocs and frees. Sorry, i don't get what you mean. how to define it? Can you elaborate? > >> + >> + =A0 =A0acb =3D qemu_aio_get(&block_queue_pool, bs, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 qemu_block_queue_callback,= opaque); >> + >> + =A0 =A0request->acb =3D acb; >> + >> + =A0 =A0return acb; >> +} >> + >> +int qemu_block_queue_handler(BlockIORequest *request) > > This function can be static. Sure. > > Stefan > --=20 Regards, Zhi Yong Wu