Ryan Harper wrote:
* Anthony Liguori <[EMAIL PROTECTED]> [2008-09-22 21:52]:
Ryan Harper wrote:
This was taken from block-raw-posix.c, DEBUG_BLOCK. I'll change up the
DEBUG_BLOCK_AIO, should I also submit a patch to rework DEBUG_BLOCK?
Yes, please.
+typedef struct AIODriver
+{
+ const char *name;
+ RawAIOCB *(*submit)(BlockDriverState *bs, int fd,
+ int64_t sector_num, uint8_t *buf,
+ int sectors, int write,
+ BlockDriverCompletionFunc *cb,
+ void *opaque);
+ void (*cancel)(BlockDriverAIOCB *aiocb);
+ int (*flush)(void *opaque);
+} AIODriver;
I think the AIODriver interface should just take an fd, a completion
function, and an opaque pointer. It should have to have knowledge of
BDRVRawState or BlockDriverState.
I assume you mean shouldn't have to have. Looking at things like
pa_read() in aio-posix.c , I'm not sure I see how I avoid that
knowledge.
I think the key is to change the way AIOCBs are allocated.
*raw_aio_read(BlockDriverState *bs,
}
#endif
- acb = raw_aio_setup(bs, sector_num, buf, nb_sectors, cb, opaque);
- if (!acb)
+ if (fd_open(bs) < 0)
return NULL;
- if (aio_read(&acb->aiocb) < 0) {
- qemu_aio_release(acb);
+
+ /* submit read */
+ acb = s->aio_dvr->submit(bs, s->fd, sector_num, buf, nb_sectors, 0,
cb,
+ opaque);
+ if (!acb)
return NULL;
- }
return &acb->common;
}
So what happens if !defined(CONFIG_AIO)? By my reading of the code,
aio_drv will be NULL and this will SEGV.
raw_aio_read/write/cancel aren't included in the bdrv structure unless
CONFIG_AIO is defined. Rather in bdrv_register, the aio emulation
functions are used instead.
So these will give warnings then of unused statics? Because they are no
longer conditional on CONFIG_AIO?
Regards,
Anthony Liguori
+ /* init aio driver for this block device */
+ s->aio_dvr = posix_aio_init();
Doesn't this need to be conditional on CONFIG_AIO?
Yep, in both raw_open and hdev_open().
Thanks for the review.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
