Hi again, Thanks for looking through the patches. I'm just going through and making the suggested changes now. I've also got qemu-nbd and block/nbd.c working over IPv6 :) - hopefully I'll be able to provide patches in a couple of days. Just a few questions about some of the changes...
Canceled requests: > > + > > + > > +static void nbd_aio_cancel(BlockDriverAIOCB *blockacb) > > +{ > > + NBDAIOCB *acb = (NBDAIOCB *)blockacb; > > + > > + /* > > + * We cannot cancel the requests which are already sent to > > + * the servers, so we just complete the request with -EIO here. > > + */ > > + acb->common.cb(acb->common.opaque, -EIO); > > + acb->canceled = 1; > > +} > > I think you need to check for acb->canceled before you write to the > associated buffer when receiving the reply for a read request. The > buffer might not exist any more after the request is cancelled. I "borrowed" this code from block/sheepdog.c (along with a fair few other bits ;) ) - which doesn't seem to do any special checking for cancelled write requests. So if there is a potential SIGSEGV here, I guess sheepdog is also vulnerable. Presumably, in aio_read_response, I'd need to allocate a buffer of the appropriate size, read the data from the socket, then deallocate the buffer? Or would fsetpos(fd, ftell(fd)+data_len) be sufficient? qemu-io doesn't seem to have any provisions for testing this particular code path - can you think of any tests I could run to ensure correctness? I've no idea under what situations an IO request might be cancelled. > > +static BlockDriverAIOCB *nbd_aio_readv(BlockDriverState *bs, > > + int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, > > + BlockDriverCompletionFunc *cb, void *opaque) > > +{ > > [...] > > + for (i = 0; i < qiov->niov; i++) { > > + memset(qiov->iov[i].iov_base, 0, qiov->iov[i].iov_len); > > + } > > qemu_iovec_memset? > > What is this even for? Aren't these zeros overwritten anyway? Again, present in sheepdog - but it does seem to work fine without it. I'll remove it from NBD.