From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:49243) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R0Ueb-0001qE-Jg for qemu-devel@nongnu.org; Mon, 05 Sep 2011 04:35:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R0UeZ-0001pe-Dr for qemu-devel@nongnu.org; Mon, 05 Sep 2011 04:35:41 -0400 Received: from e23smtp02.au.ibm.com ([202.81.31.144]:58954) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R0UeY-0001p7-D9 for qemu-devel@nongnu.org; Mon, 05 Sep 2011 04:35:39 -0400 Received: from d23relay04.au.ibm.com (d23relay04.au.ibm.com [202.81.31.246]) by e23smtp02.au.ibm.com (8.14.4/8.13.1) with ESMTP id p858SwGn030846 for ; Mon, 5 Sep 2011 18:28:58 +1000 Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p858XqHt802966 for ; Mon, 5 Sep 2011 18:33:52 +1000 Received: from d23av04.au.ibm.com (loopback [127.0.0.1]) by d23av04.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p858ZNSq001249 for ; Mon, 5 Sep 2011 18:35:23 +1000 Date: Mon, 5 Sep 2011 16:34:26 +0800 From: Zhi Yong Wu Message-ID: <20110905083426.GJ19143@f15.cn.ibm.com> References: <1314877456-19521-1-git-send-email-wuzhy@linux.vnet.ibm.com> <1314877456-19521-3-git-send-email-wuzhy@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Description:  Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v6 2/4] block: add the block queue support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: qemu-devel@nongnu.org On Thu, Sep 01, 2011 at 02:02:41PM +0100, Stefan Hajnoczi wrote: > 01 Sep 2011 06:02:41 -0700 (PDT) >Received: by 10.220.200.77 with HTTP; Thu, 1 Sep 2011 06:02:41 -0700 (PD= T) >In-Reply-To: <1314877456-19521-3-git-send-email-wuzhy@linux.vnet.ibm.com= > >References: <1314877456-19521-1-git-send-email-wuzhy@linux.vnet.ibm.com> > <1314877456-19521-3-git-send-email-wuzhy@linux.vnet.ibm.com> >Date: Thu, 1 Sep 2011 14:02:41 +0100 >Message-ID: >From: Stefan Hajnoczi >To: Zhi Yong Wu >Content-Type: text/plain; charset=3DISO-8859-1 >Content-Transfer-Encoding: quoted-printable >X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) >X-Received-From: 209.85.220.173 >Cc: kwolf@redhat.com, aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com, > kvm@vger.kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org, > pair@us.ibm.com, zwu.kernel@gmail.com, ryanh@us.ibm.com >Subject: Re: [Qemu-devel] [PATCH v6 2/4] block: add the block queue supp= ort >X-BeenThere: qemu-devel@nongnu.org >X-Mailman-Version: 2.1.14 >Precedence: list >List-Id: >List-Unsubscribe: , > >List-Archive: >List-Post: >List-Help: >List-Subscribe: , > >X-Mailman-Copy: yes >Errors-To: qemu-devel-bounces+wuzhy=3Dlinux.vnet.ibm.com@nongnu.org >Sender: qemu-devel-bounces+wuzhy=3Dlinux.vnet.ibm.com@nongnu.org >X-Brightmail-Tracker: AAAAAA=3D=3D >X-Xagent-From: stefanha@gmail.com >X-Xagent-To: wuzhy@linux.vnet.ibm.com >X-Xagent-Gateway: vmsdvma.vnet.ibm.com (XAGENTU4 at VMSDVMA) > >On Thu, Sep 1, 2011 at 12:44 PM, Zhi Yong Wu = wrote: >> +struct BlockIORequest { >> + =C2=A0 =C2=A0QTAILQ_ENTRY(BlockIORequest) entry; >> + =C2=A0 =C2=A0BlockDriverState *bs; >> + =C2=A0 =C2=A0BlockRequestHandler *handler; >> + =C2=A0 =C2=A0int64_t sector_num; >> + =C2=A0 =C2=A0QEMUIOVector *qiov; >> + =C2=A0 =C2=A0int nb_sectors; >> + =C2=A0 =C2=A0BlockDriverCompletionFunc *cb; >> + =C2=A0 =C2=A0void *opaque; >> + =C2=A0 =C2=A0BlockDriverAIOCB *acb; >> +}; >> + >> +struct BlockQueue { >> + =C2=A0 =C2=A0QTAILQ_HEAD(requests, BlockIORequest) requests; >> + =C2=A0 =C2=A0BlockIORequest *request; >> + =C2=A0 =C2=A0bool flushing; >> +}; >> + >> +struct BlockQueueAIOCB { >> + =C2=A0 =C2=A0BlockDriverAIOCB common; >> + =C2=A0 =C2=A0BlockDriverCompletionFunc *real_cb; >> + =C2=A0 =C2=A0BlockDriverAIOCB *real_acb; >> + =C2=A0 =C2=A0void *opaque; >> + =C2=A0 =C2=A0BlockIORequest *request; >> +}; > >Now it's pretty easy to merge BlockIORequest and BlockQueueAIOCB into >one struct. That way we don't need to duplicate fields or link >structures together: Must we merge them? I think that it will cause the logic is not clear tha= n current way. It is weird that some BlockQueueAIOCB are linked to block = queue although it reduce the LOC to some extent. Moreover, those block-queue functions need to be rewritten. > >typedef struct BlockQueueAIOCB { > BlockDriverAIOCB common; > QTAILQ_ENTRY(BlockQueueAIOCB) entry; /* held requests */ > BlockRequestHandler *handler; /* dispatch function *= / > BlockDriverAIOCB *real_acb; /* dispatched aiocb */ > > /* Request parameters */ > int64_t sector_num; > QEMUIOVector *qiov; > int nb_sectors; >} BlockQueueAIOCB; > >This struct is kept for the lifetime of a request (both during >queueing and after dispatch). ditto. > >> + >> +static void qemu_block_queue_dequeue(BlockQueue *queue, BlockIOReques= t *request) >> +{ >> + =C2=A0 =C2=A0BlockIORequest *req; >> + >> + =C2=A0 =C2=A0while (!QTAILQ_EMPTY(&queue->requests)) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0req =3D QTAILQ_FIRST(&queue->requests); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (req =3D=3D request) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0QTAILQ_REMOVE(&queue->reque= sts, req, entry); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0} >> + =C2=A0 =C2=A0} >> +} >> + >> +static void qemu_block_queue_cancel(BlockDriverAIOCB *acb) >> +{ >> + =C2=A0 =C2=A0BlockQueueAIOCB *blkacb =3D container_of(acb, BlockQueu= eAIOCB, common); >> + =C2=A0 =C2=A0if (blkacb->real_acb) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0bdrv_aio_cancel(blkacb->real_acb); >> + =C2=A0 =C2=A0} else { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0assert(blkacb->common.bs->block_queue); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_block_queue_dequeue(blkacb->common.b= s->block_queue, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 blkacb->request); > >Can we use QTAILQ_REMOVE() and eliminate qemu_block_queue_dequeue()? why need to replace dequeue function with QTAILQ_REMOVE()? >If the aiocb exists and is not dispatched (real_acb !=3D NULL) then it >must be queued. Can you explain clearlier? > >> + =C2=A0 =C2=A0} >> + >> + =C2=A0 =C2=A0qemu_aio_release(blkacb); >> +} >> + >> +static AIOPool block_queue_pool =3D { >> + =C2=A0 =C2=A0.aiocb_size =C2=A0 =C2=A0 =C2=A0 =C2=A0 =3D sizeof(stru= ct BlockQueueAIOCB), >> + =C2=A0 =C2=A0.cancel =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =3D q= emu_block_queue_cancel, >> +}; >> + >> +static void qemu_block_queue_callback(void *opaque, int ret) >> +{ >> + =C2=A0 =C2=A0BlockQueueAIOCB *acb =3D opaque; >> + >> + =C2=A0 =C2=A0if (acb->real_cb) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0acb->real_cb(acb->opaque, ret); >> + =C2=A0 =C2=A0} >> + >> + =C2=A0 =C2=A0qemu_aio_release(acb); >> +} >> + >> +BlockQueue *qemu_new_block_queue(void) >> +{ >> + =C2=A0 =C2=A0BlockQueue *queue; >> + >> + =C2=A0 =C2=A0queue =3D g_malloc0(sizeof(BlockQueue)); >> + >> + =C2=A0 =C2=A0QTAILQ_INIT(&queue->requests); >> + >> + =C2=A0 =C2=A0queue->flushing =3D false; >> + =C2=A0 =C2=A0queue->request =C2=A0=3D NULL; >> + >> + =C2=A0 =C2=A0return queue; >> +} >> + >> +void qemu_del_block_queue(BlockQueue *queue) >> +{ >> + =C2=A0 =C2=A0BlockIORequest *request, *next; >> + >> + =C2=A0 =C2=A0QTAILQ_FOREACH_SAFE(request, &queue->requests, entry, n= ext) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0QTAILQ_REMOVE(&queue->requests, request, = entry); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0g_free(request); > >What about the acbs? There needs to be a strategy for cleanly what strategy? >shutting down completely. In fact we should probably use cancellation >here or assert that the queue is already empty. You mean if the queue has been empty, we need to assert(queue)? > >> + =C2=A0 =C2=A0} >> + >> + =C2=A0 =C2=A0g_free(queue); >> +} >> + >> +BlockDriverAIOCB *qemu_block_queue_enqueue(BlockQueue *queue, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0BlockDriverState *bs, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0BlockRequestHandler *handler, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0int64_t sector_num, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0QEMUIOVector *qiov, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0int nb_sectors, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0BlockDriverCompletionFunc *cb, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0void *opaque) >> +{ >> + =C2=A0 =C2=A0BlockIORequest *request; >> + =C2=A0 =C2=A0BlockDriverAIOCB *acb; >> + =C2=A0 =C2=A0BlockQueueAIOCB *blkacb; >> + >> + =C2=A0 =C2=A0if (queue->request) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0request =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =3D queue->request; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0assert(request->acb); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0acb =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =3D request->acb; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0QTAILQ_INSERT_TAIL(&queue->requests, requ= est, entry); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0queue->request =C2=A0 =C2=A0 =C2=A0=3D NU= LL; > >I don't think we need this behavior. There is already requeuing logic >in the flush function: if request->handler() fails then the request is >put back onto the queue and we stop flushing. > >So all we need here is: > >if (queue->flushing) { > return NULL; >} very nice, thanks. > >This results in the request->handler() failing (returning NULL) and >the flush function then requeues this request. > >In other words, during flushing we do not allow any new requests to be e= nqueued. > >> + =C2=A0 =C2=A0} else { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0request =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =3D g_malloc0(sizeof(BlockIORequest)); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0request->bs =C2=A0 =C2=A0 =C2=A0 =C2=A0 =3D= bs; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0request->handler =C2=A0 =C2=A0=3D handler= ; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0request->sector_num =3D sector_num; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0request->qiov =C2=A0 =C2=A0 =C2=A0 =3D qi= ov; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0request->nb_sectors =3D nb_sectors; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0request->cb =C2=A0 =C2=A0 =C2=A0 =C2=A0 =3D= qemu_block_queue_callback; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0QTAILQ_INSERT_TAIL(&queue->requests, requ= est, entry); >> + >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0acb =3D qemu_aio_get(&block_queue_pool, b= s, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 qemu_block_queue_callback, opaque); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0blkacb =3D container_of(acb, BlockQueueAI= OCB, common); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0blkacb->real_cb =C2=A0 =C2=A0 =3D cb; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0blkacb->real_acb =C2=A0 =C2=A0=3D NULL; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0blkacb->opaque =C2=A0 =C2=A0 =C2=A0=3D op= aque; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0blkacb->request =C2=A0 =C2=A0 =3D request= ; >> + >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0request->acb =C2=A0 =C2=A0 =C2=A0 =C2=A0=3D= acb; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0request->opaque =3D (void *)blkacb; > >Here's what this initialization code looks like when BlockIORequest >and BlockQueueAIOCB are merged: > >acb =3D qemu_aio_get(&block_queue_pool, bs, cb, opaque); >acb->handler =C2=A0 =C2=A0=3D handler; >acb->sector_num =3D sector_num; >acb->qiov =C2=A0 =C2=A0 =C2=A0 =3D qiov; >acb->nb_sectors =3D nb_sectors; >acb->real_acb=C2=A0 =C2=A0=3D NULL; >QTAILQ_INSERT_TAIL(&queue->requests, acb, entry); pls see comment #1. > >> + =C2=A0 =C2=A0} >> + >> + =C2=A0 =C2=A0return acb; >> +} >> + >> +static int qemu_block_queue_handler(BlockIORequest *request) >> +{ >> + =C2=A0 =C2=A0int ret; >> + =C2=A0 =C2=A0BlockDriverAIOCB *res; >> + >> + =C2=A0 =C2=A0res =3D request->handler(request->bs, request->sector_n= um, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 request->qiov, request->nb_sectors, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 request->cb, request->opaque); > >Please pass qemu_block_queue_callback and request->acb directly here >instead of using request->cb and request->opaque. Those fields aren't >needed and just add indirection. If later the other guy want to reuse qemu_block_queue_handler, and use di= fferent callback function, then this function can not be used. Your way l= ower this function's reusability. > >> + =C2=A0 =C2=A0if (res) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0BlockQueueAIOCB *blkacb =3D >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0container_of(= request->acb, BlockQueueAIOCB, common); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0blkacb->real_acb =3D res; >> + =C2=A0 =C2=A0} >> + >> + =C2=A0 =C2=A0ret =3D (res =3D=3D NULL) ? 0 : 1; >> + >> + =C2=A0 =C2=A0return ret; >> +} >> + >> +void qemu_block_queue_flush(BlockQueue *queue) >> +{ >> + =C2=A0 =C2=A0queue->flushing =3D true; >> + =C2=A0 =C2=A0while (!QTAILQ_EMPTY(&queue->requests)) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0BlockIORequest *request =3D NULL; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0int ret =3D 0; >> + >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0request =3D QTAILQ_FIRST(&queue->requests= ); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0QTAILQ_REMOVE(&queue->requests, request, = entry); >> + >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0queue->request =3D request; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0ret =3D qemu_block_queue_handler(request)= ; >> + >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (ret =3D=3D 0) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0QTAILQ_INSERT_HEAD(&queue->= requests, request, entry); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0queue->request =3D NULL; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0} >> + >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (queue->request) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0g_free(request); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0} >> + >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0queue->request =3D NULL; >> + =C2=A0 =C2=A0} >> + >> + =C2=A0 =C2=A0queue->flushing =3D false; >> +} >> + >> +bool qemu_block_queue_has_pending(BlockQueue *queue) >> +{ >> + =C2=A0 =C2=A0return !queue->flushing && !QTAILQ_EMPTY(&queue->reques= ts); >> +} >> diff --git a/block/blk-queue.h b/block/blk-queue.h >> new file mode 100644 >> index 0000000..84c2407 >> --- /dev/null >> +++ b/block/blk-queue.h >> @@ -0,0 +1,63 @@ >> +/* >> + * QEMU System Emulator queue declaration for block layer >> + * >> + * Copyright (c) IBM, Corp. 2011 >> + * >> + * Authors: >> + * =C2=A0Zhi Yong Wu =C2=A0 >> + * =C2=A0Stefan Hajnoczi > >Please use linux.vnet.ibm.com addresses and not GMail. > >> + * >> + * Permission is hereby granted, free of charge, to any person obtain= ing a copy >> + * of this software and associated documentation files (the "Software= "), to deal >> + * in the Software without restriction, including without limitation = the rights >> + * to use, copy, modify, merge, publish, distribute, sublicense, and/= or sell >> + * copies of the Software, and to permit persons to whom the Software= is >> + * furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be inc= luded in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EX= PRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTAB= ILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT = SHALL >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES = OR OTHER >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, AR= ISING FROM, >> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEAL= INGS IN >> + * THE SOFTWARE. >> + */ >> + >> +#ifndef QEMU_BLOCK_QUEUE_H >> +#define QEMU_BLOCK_QUEUE_H >> + >> +#include "block.h" >> +#include "qemu-queue.h" >> + >> +typedef BlockDriverAIOCB* (BlockRequestHandler) (BlockDriverState *bs= , >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0int64_t sector_num, QEMUIOVecto= r *qiov, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0int nb_sectors, BlockDriverComp= letionFunc *cb, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0void *opaque); >> + >> +typedef struct BlockIORequest BlockIORequest; > >BlockIORequest does not need a forward declaration because only >blk-queue.c uses it. It's a private type that the rest of QEMU should >not know about. OK. > >> + >> +typedef struct BlockQueue BlockQueue; >> + >> +typedef struct BlockQueueAIOCB BlockQueueAIOCB; > >This is also a private type, callers only know about BlockDriverAIOCB. > Please move to blk-queue.c. ditto > >Stefan >