Am 03.06.2016 um 19:03 hat Eric Blake geschrieben: > It makes more sense to have ALL block size limit constraints > in the same struct. Improve the documentation while at it. > > Note that bdrv_refresh_limits() has to keep things alive across > a memset() of BlockLimits. > > Signed-off-by: Eric Blake <ebl...@redhat.com> > --- > include/block/block_int.h | 12 ++++++++---- > block.c | 4 ++-- > block/blkdebug.c | 4 ++-- > block/bochs.c | 2 +- > block/cloop.c | 2 +- > block/dmg.c | 2 +- > block/io.c | 12 +++++++----- > block/iscsi.c | 2 +- > block/raw-posix.c | 16 ++++++++-------- > block/raw-win32.c | 6 +++--- > block/vvfat.c | 2 +- > 11 files changed, 35 insertions(+), 29 deletions(-) > > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 9dc7af4..0bc7ff4 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -322,6 +322,12 @@ struct BlockDriver { > }; > > typedef struct BlockLimits { > + /* Alignment requirement, in bytes, for offset/length of I/O > + * requests. Must be a power of 2, and should not exceed > + * INT_MAX. For now, a value of 0 defaults to 512, but may change > + * to default to 1. */ > + uint32_t request_alignment; > + > /* maximum number of bytes that can be discarded at once (since it > * is signed, it must be < 2G, if set), should be multiple of > * pdiscard_alignment, but need not be power of 2. May be 0 if no > @@ -353,10 +359,10 @@ typedef struct BlockLimits { > * should be multiple of opt_transfer), or 0 for no 32-bit limit */ > uint32_t max_transfer; > > - /* memory alignment so that no bounce buffer is needed */ > + /* memory alignment, in bytes so that no bounce buffer is needed */ > size_t min_mem_alignment; > > - /* memory alignment for bounce buffer */ > + /* memory alignment, in bytes, for bounce buffer */ > size_t opt_mem_alignment; > > /* maximum number of iovec elements */ > @@ -463,8 +469,6 @@ struct BlockDriverState { > /* Whether produces zeros when read beyond eof */ > bool zero_beyond_eof; > > - /* Alignment requirement for offset/length of I/O requests */ > - unsigned int request_alignment; > /* Flags honored during pwrite (so far: BDRV_REQ_FUA) */ > unsigned int supported_write_flags; > /* Flags honored during pwrite_zeroes (so far: BDRV_REQ_FUA, > diff --git a/block.c b/block.c > index f54bc25..1b56161 100644 > --- a/block.c > +++ b/block.c > @@ -937,7 +937,7 @@ static int bdrv_open_common(BlockDriverState *bs, > BdrvChild *file, > goto fail_opts; > } > > - bs->request_alignment = 512; > + bs->bl.request_alignment = 512; > bs->zero_beyond_eof = true; > bs->read_only = !(bs->open_flags & BDRV_O_RDWR);
We don't explicitly initialise any other BlockLimits here, the function calls bdrv_refresh_limits() to do that. > diff --git a/block/io.c b/block/io.c > index e845ef9..aaeedaa 100644 > --- a/block/io.c > +++ b/block/io.c > @@ -71,8 +71,10 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error > **errp) > { > BlockDriver *drv = bs->drv; > Error *local_err = NULL; > + uint32_t request_alignment = bs->bl.request_alignment; > > memset(&bs->bl, 0, sizeof(bs->bl)); > + bs->bl.request_alignment = request_alignment; This looks fishy, and it's because you didn't fully convert request_alignment to work like all the other limits. You should set the default of 512 here in bdrv_refresh_limits() before calling into drv->bdrv_refresh_limits() where it can be overridden. No other fields are set earlier and then preserved here, we start over with everything. Of course, this means that drivers that set request_alignment must be changed so that they don't set it in .bdrv_open(), but only in their .bdrv_refresh_limits() callback. Kevin