On 12.10.20 19:43, Andrey Shinkevich wrote:
Limit COR operations by the base node in the backing chain when the
overlay base node name is given. It will be useful for a block stream
job when the COR-filter is applied. The overlay base node is passed as
the base itself may change due to concurrent commit jobs on the same
backing chain.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich(a)virtuozzo.com>
---
block/copy-on-read.c | 39 +++++++++++++++++++++++++++++++++++++--
1 file changed, 37 insertions(+), 2 deletions(-)
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index c578b1b..dfbd6ad 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -122,8 +122,43 @@ static int coroutine_fn cor_co_preadv_part(BlockDriverState *bs,
size_t qiov_offset,
int flags)
{
- return bdrv_co_preadv_part(bs->file, offset, bytes, qiov, qiov_offset,
- flags | BDRV_REQ_COPY_ON_READ);
+ int64_t n = 0;
+ int64_t size = offset + bytes;
+ int local_flags;
+ int ret;
+ BDRVStateCOR *state = bs->opaque;
+
+ if (!state->base_overlay) {
+ return bdrv_co_preadv_part(bs->file, offset, bytes, qiov, qiov_offset,
+ flags | BDRV_REQ_COPY_ON_READ);
+ }
+
+ while (offset < size) {
+ local_flags = flags;
+
+ /* In case of failure, try to copy-on-read anyway */
+ ret = bdrv_is_allocated(bs->file->bs, offset, bytes, &n);
+ if (!ret) {
In case of failure, a negative value is going to be returned, we won’t
go into this conditional block, and local_flags isn’t going to contain
BDRV_REQ_COPY_ON_READ.
So the idea of CORing in case of failure sounds sound to me, but it
doesn’t look like that’s done.
+ ret =
bdrv_is_allocated_above(bdrv_cow_bs(bs->file->bs),
I think this should either be bdrv_backing_chain_next() or we must rule
out the possibility of bs->file->bs being a filter somewhere. I think
I’d prefer the former.
+ state->base_overlay,
true, offset,
+ n, &n);
+ if (ret) {
“ret == 1 || ret < 0” would be more explicit (and in line with the “!ret
|| ret < 0” probably needed above), but correct either way.
Max