diff mbox series

[v10,5/9] copy-on-read: limit guest COR activity to base in COR driver

Message ID 1601383109-110988-6-git-send-email-andrey.shinkevich@virtuozzo.com
State Superseded
Headers show
Series Apply COR-filter to the block-stream permanently | expand

Commit Message

Xingtao Yao (Fujitsu)" via Sept. 29, 2020, 12:38 p.m. UTC
Limit the guest's COR operations by the base node in the backing chain
when the base node name is given. It will be useful for a block stream
job when the COR-filter is applied.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 block/copy-on-read.c | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

Comments

Vladimir Sementsov-Ogievskiy Oct. 5, 2020, 2:58 p.m. UTC | #1
29.09.2020 15:38, Andrey Shinkevich wrote:
> Limit the guest's COR operations by the base node in the backing chain
> when the base node name is given. It will be useful for a block stream
> job when the COR-filter is applied.
> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   block/copy-on-read.c | 38 ++++++++++++++++++++++++++++++++++++--
>   1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/block/copy-on-read.c b/block/copy-on-read.c
> index e04092f..f53f7e0 100644
> --- a/block/copy-on-read.c
> +++ b/block/copy-on-read.c
> @@ -121,8 +121,42 @@ 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_bs) {
> +        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 */

But you add the flag only in case of success.. On any failure of furhter is*allocated calls we should set the flag.

> +        ret = bdrv_is_allocated(bs->file->bs, offset, bytes, &n);
> +        if (!ret) {
> +            ret = bdrv_is_allocated_above(bdrv_cow_bs(bs->file->bs),
> +                                          state->base_bs, false, offset, n, &n);
> +            if (ret > 0) {
> +                local_flags |= BDRV_REQ_COPY_ON_READ;
> +            }
> +        }
> +
> +        ret = bdrv_co_preadv_part(bs->file, offset, n, qiov, qiov_offset,
> +                                  local_flags);
> +        if (ret < 0) {
> +            return ret;
> +        }
> +
> +        offset += n;
> +        qiov_offset += n;
> +        bytes -= n;
> +    }
> +
> +    return 0;
>   }
>   
>   
>
Andrey Shinkevich Oct. 5, 2020, 4:45 p.m. UTC | #2
On 05.10.2020 17:58, Vladimir Sementsov-Ogievskiy wrote:
> 29.09.2020 15:38, Andrey Shinkevich wrote:
>> Limit the guest's COR operations by the base node in the backing chain
>> when the base node name is given. It will be useful for a block stream
>> job when the COR-filter is applied.
>>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>>   block/copy-on-read.c | 38 ++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/copy-on-read.c b/block/copy-on-read.c
>> index e04092f..f53f7e0 100644
>> --- a/block/copy-on-read.c
>> +++ b/block/copy-on-read.c
>> @@ -121,8 +121,42 @@ 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_bs) {
>> +        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 */
> 
> But you add the flag only in case of success.. On any failure of furhter 
> is*allocated calls we should set the flag.

Yes, thanks.
Andrey

> 
>> +        ret = bdrv_is_allocated(bs->file->bs, offset, bytes, &n);
>> +        if (!ret) {
>> +            ret = bdrv_is_allocated_above(bdrv_cow_bs(bs->file->bs),
>> +                                          state->base_bs, false, 
>> offset, n, &n);
>> +            if (ret > 0) {
>> +                local_flags |= BDRV_REQ_COPY_ON_READ;
>> +            }
>> +        }
>> +
>> +        ret = bdrv_co_preadv_part(bs->file, offset, n, qiov, 
>> qiov_offset,
>> +                                  local_flags);
>> +        if (ret < 0) {
>> +            return ret;
>> +        }
>> +
>> +        offset += n;
>> +        qiov_offset += n;
>> +        bytes -= n;
>> +    }
>> +
>> +    return 0;
>>   }
>>
> 
>
Andrey Shinkevich Oct. 5, 2020, 6:18 p.m. UTC | #3
On 05.10.2020 17:58, Vladimir Sementsov-Ogievskiy wrote:
> 29.09.2020 15:38, Andrey Shinkevich wrote:
>> Limit the guest's COR operations by the base node in the backing chain
>> when the base node name is given. It will be useful for a block stream
>> job when the COR-filter is applied.
>>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>>   block/copy-on-read.c | 38 ++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/copy-on-read.c b/block/copy-on-read.c
>> index e04092f..f53f7e0 100644
>> --- a/block/copy-on-read.c
>> +++ b/block/copy-on-read.c
>> @@ -121,8 +121,42 @@ 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_bs) {
>> +        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 */
> 
> But you add the flag only in case of success.. On any failure of furhter 
> is*allocated calls we should set the flag.
> 

Actually, myself would prefer returning the error instead.

Andrey

>> +        ret = bdrv_is_allocated(bs->file->bs, offset, bytes, &n);
>> +        if (!ret) {
>> +            ret = bdrv_is_allocated_above(bdrv_cow_bs(bs->file->bs),
>> +                                          state->base_bs, false, 
>> offset, n, &n);
>> +            if (ret > 0) {
>> +                local_flags |= BDRV_REQ_COPY_ON_READ;
>> +            }
>> +        }
>> +
>> +        ret = bdrv_co_preadv_part(bs->file, offset, n, qiov, 
>> qiov_offset,
>> +                                  local_flags);
>> +        if (ret < 0) {
>> +            return ret;
>> +        }
>> +
>> +        offset += n;
>> +        qiov_offset += n;
>> +        bytes -= n;
>> +    }
>> +
>> +    return 0;
>>   }
>>
> 
>
diff mbox series

Patch

diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index e04092f..f53f7e0 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -121,8 +121,42 @@  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_bs) {
+        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) {
+            ret = bdrv_is_allocated_above(bdrv_cow_bs(bs->file->bs),
+                                          state->base_bs, false, offset, n, &n);
+            if (ret > 0) {
+                local_flags |= BDRV_REQ_COPY_ON_READ;
+            }
+        }
+
+        ret = bdrv_co_preadv_part(bs->file, offset, n, qiov, qiov_offset,
+                                  local_flags);
+        if (ret < 0) {
+            return ret;
+        }
+
+        offset += n;
+        qiov_offset += n;
+        bytes -= n;
+    }
+
+    return 0;
 }