[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Re: [PATCH 3/3] block/nbd: Make the NBD block device us
From: |
Nicholas Thomas |
Subject: |
Re: [Qemu-devel] Re: [PATCH 3/3] block/nbd: Make the NBD block device use the AIO interface |
Date: |
Tue, 22 Feb 2011 11:48:31 +0000 |
> > + * Send I/O requests to the server.
> > + *
> > + * This function sends requests to the server, links the requests to
> > + * the outstanding_list in BDRVNBDState, and exits without waiting for
> > + * the response. The responses are received in the `aio_read_response'
> > + * function which is called from the main loop as a fd handler.
> > + * If this is a write request and it's >1MB, split it into multiple AIOReqs
> > + */
> > +static void nbd_readv_writev_bh_cb(void *p)
> > {
> > - BDRVNBDState *s = bs->opaque;
> > - struct nbd_request request;
> > + NBDAIOCB *acb = p;
> > + int ret = 0;
> >
> > - request.type = NBD_CMD_DISC;
> > - request.handle = (uint64_t)(intptr_t)bs;
> > - request.from = 0;
> > - request.len = 0;
> > - nbd_send_request(s->sock, &request);
> > + size_t len, done = 0;
> > + size_t total = acb->nb_sectors * SECTOR_SIZE;
> > +
> > + /* Where the read/write starts from */
> > + size_t offset = acb->sector_num * SECTOR_SIZE;
>
> On 32-bit hosts size_t is only 32 bits wide. It's not suitable for
> storing the byte offset into the device. Please use off_t.
>
> > + BDRVNBDState *s = acb->common.bs->opaque;
>
> I don't follow this. You have no guarantee that acb->common.bs->opaque
> is the BDRVNBDState *.
Another idiom from sheepdog. Seems qed, qcow2, qcow & vdi do the same.
It seems the magic is being done in nbd_aio_setup & qemu_aio_setup
(gdb) bt
#0 nbd_aio_setup (bs=0xa69600, qiov=0x7fffffffdc00, sector_num=0,
nb_sectors=4, cb=0x4041c6 <aio_rw_done>,
opaque=0x7fffffffdbac) at block/nbd.c:443
#1 0x00000000004395c3 in nbd_aio_readv (bs=0xa69600, sector_num=0,
qiov=0x7fffffffdc00, nb_sectors=4,
cb=0x4041c6 <aio_rw_done>, opaque=0x7fffffffdbac) at block/nbd.c:538
#2 0x000000000041223c in bdrv_aio_readv (bs=0xa69600, sector_num=0,
qiov=0x7fffffffdc00, nb_sectors=4,
cb=0x4041c6 <aio_rw_done>, opaque=0x7fffffffdbac) at block.c:2158
[...]
(gdb) inspect acb
$17 = (NBDAIOCB *) 0x40f708
(gdb) inspect &acb->common
$21 = (BlockDriverAIOCB *) 0x40f708
(Sorry, the line numbers are unlikely to match yours).
It's some kind of simple polymorphism? Everything south of block/nbd.c
accesses the memory region as a BlockDriverAIOCB, while everything north
accesses it as an NBDAIOCB - with the additional fields that implies?
I've never come across something like this before - but it does seem to
work.
acb->common.bs is set to the BlockDriverState we use everywhere - whose
opaque field is known. In what situations will this fail to work?
/Nick