[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH for-2.6 2/2] block: Fix blk_aio_wri
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH for-2.6 2/2] block: Fix blk_aio_write_zeroes() |
Date: |
Fri, 15 Apr 2016 10:34:09 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 15.04.2016 um 01:30 hat Max Reitz geschrieben:
> On 13.04.2016 13:06, Kevin Wolf wrote:
> > Commit 57d6a428 broke blk_aio_write_zeroes() because in some write
> > functions in the call path don't have an explicit length argument but
> > reuse qiov->size instead. Which is great, except that write_zeroes
> > doesn't have a qiov, which this commit interprets as 0 bytes.
> > Consequently, blk_aio_write_zeroes() didn't effectively do anything.
>
> You mean it has written null data? :-)
>
> Alternative version: So it has written zero bytes? Works as intended, then.
I consciously avoided the latter wording. ;-)
> > This patch introduces an explicit acb->bytes in BlkAioEmAIOCB and uses
> > that instead of acb->rwco.size.
> >
> > The synchronous version of the function is okay because it does pass a
> > qiov (with the right size and a NULL pointer as its base).
> >
> > Signed-off-by: Kevin Wolf <address@hidden>
> > @@ -937,7 +940,8 @@ BlockAIOCB *blk_aio_write_zeroes(BlockBackend *blk,
> > int64_t sector_num,
> > return blk_abort_aio_request(blk, cb, opaque, -EINVAL);
> > }
> >
> > - return blk_aio_prwv(blk, sector_num << BDRV_SECTOR_BITS, NULL,
> > + return blk_aio_prwv(blk, sector_num << BDRV_SECTOR_BITS,
> > + nb_sectors << BDRV_SECTOR_BITS, NULL,
> > blk_aio_write_entry, BDRV_REQ_ZERO_WRITE, cb,
> > opaque);
> > }
>
> Another thing about blk_aio_write_zeroes() that might need fixing is
> that it completely ignores its flags parameter.
>
> That can be done in a seperate patch, however (and is probably not
> critical, as this currently just means that a potential REQ_MAY_UNMAP
> from scsi-disk is ignored), so:
Oh dear... Commit 57d6a428 was not my best patch ever, and apparently
Stefan wasn't suspicious enough to catch the bugs. Maybe someone should
re-review the whole thing...
"just" ignoring REQ_MAY_UNMAP is a regression in discard support, so no,
that's not what I would call harmless. Definitely needs fixing for 2.6.
And we probably need more qtests checking such things.
Kevin
pgpkdk7aVRiHa.pgp
Description: PGP signature