| Message ID | 20201204220758.2879-8-vsementsov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Apply COR-filter to the block-stream permanently | expand |
On 04.12.20 23:07, Vladimir Sementsov-Ogievskiy wrote: > From: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> > > Add the new member supported_read_flags to the BlockDriverState > structure. It will control the flags set for copy-on-read operations. > Make the block generic layer evaluate supported read flags before they > go to a block driver. > > Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > include/block/block_int.h | 4 ++++ > block/io.c | 12 ++++++++++-- > 2 files changed, 14 insertions(+), 2 deletions(-) [...] > diff --git a/block/io.c b/block/io.c > index ec5e152bb7..e28b11c42b 100644 > --- a/block/io.c > +++ b/block/io.c [...] > @@ -1426,9 +1429,13 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child, > goto out; > } > > + if (flags & ~bs->supported_read_flags) { > + abort(); > + } I’d prefer an assert(!(flags & ~bs->supported_read_flags)), so in case we do abort, there’s going to be an error message that immediately tells what the problem is. Apart from that: Reviewed-by: Max Reitz <mreitz@redhat.com> > + > max_bytes = ROUND_UP(MAX(0, total_bytes - offset), align); > if (bytes <= max_bytes && bytes <= max_transfer) { > - ret = bdrv_driver_preadv(bs, offset, bytes, qiov, qiov_offset, 0); > + ret = bdrv_driver_preadv(bs, offset, bytes, qiov, qiov_offset, flags); > goto out; > }
11.12.2020 16:20, Max Reitz wrote: > On 04.12.20 23:07, Vladimir Sementsov-Ogievskiy wrote: >> From: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> >> >> Add the new member supported_read_flags to the BlockDriverState >> structure. It will control the flags set for copy-on-read operations. >> Make the block generic layer evaluate supported read flags before they >> go to a block driver. >> >> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> include/block/block_int.h | 4 ++++ >> block/io.c | 12 ++++++++++-- >> 2 files changed, 14 insertions(+), 2 deletions(-) > > [...] > >> diff --git a/block/io.c b/block/io.c >> index ec5e152bb7..e28b11c42b 100644 >> --- a/block/io.c >> +++ b/block/io.c > > [...] > >> @@ -1426,9 +1429,13 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child, >> goto out; >> } >> + if (flags & ~bs->supported_read_flags) { >> + abort(); >> + } > > I’d prefer an assert(!(flags & ~bs->supported_read_flags)), so in case we do abort, there’s going to be an error message that immediately tells what the problem is. agree. and one-line check is shorter than three-line > > Apart from that: > > Reviewed-by: Max Reitz <mreitz@redhat.com> > >> + >> max_bytes = ROUND_UP(MAX(0, total_bytes - offset), align); >> if (bytes <= max_bytes && bytes <= max_transfer) { >> - ret = bdrv_driver_preadv(bs, offset, bytes, qiov, qiov_offset, 0); >> + ret = bdrv_driver_preadv(bs, offset, bytes, qiov, qiov_offset, flags); >> goto out; >> } >
diff --git a/include/block/block_int.h b/include/block/block_int.h index c05fa1eb6b..247e166ab6 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -873,6 +873,10 @@ struct BlockDriverState { /* I/O Limits */ BlockLimits bl; + /* + * Flags honored during pread + */ + unsigned int supported_read_flags; /* Flags honored during pwrite (so far: BDRV_REQ_FUA, * BDRV_REQ_WRITE_UNCHANGED). * If a driver does not support BDRV_REQ_WRITE_UNCHANGED, those diff --git a/block/io.c b/block/io.c index ec5e152bb7..e28b11c42b 100644 --- a/block/io.c +++ b/block/io.c @@ -1405,6 +1405,9 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child, if (flags & BDRV_REQ_COPY_ON_READ) { int64_t pnum; + /* The flag BDRV_REQ_COPY_ON_READ has reached its addressee */ + flags &= ~BDRV_REQ_COPY_ON_READ; + ret = bdrv_is_allocated(bs, offset, bytes, &pnum); if (ret < 0) { goto out; @@ -1426,9 +1429,13 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child, goto out; } + if (flags & ~bs->supported_read_flags) { + abort(); + } + max_bytes = ROUND_UP(MAX(0, total_bytes - offset), align); if (bytes <= max_bytes && bytes <= max_transfer) { - ret = bdrv_driver_preadv(bs, offset, bytes, qiov, qiov_offset, 0); + ret = bdrv_driver_preadv(bs, offset, bytes, qiov, qiov_offset, flags); goto out; } @@ -1441,7 +1448,8 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child, ret = bdrv_driver_preadv(bs, offset + bytes - bytes_remaining, num, qiov, - qiov_offset + bytes - bytes_remaining, 0); + qiov_offset + bytes - bytes_remaining, + flags); max_bytes -= num; } else { num = bytes_remaining;