diff mbox series

[06/14] block-backend: use bdrv_getlength instead of blk_getlength

Message ID 20221213085320.95673-7-kwolf@redhat.com
State New
Headers show
Series block: Move more functions to coroutines | expand

Commit Message

Kevin Wolf Dec. 13, 2022, 8:53 a.m. UTC
From: Emanuele Giuseppe Esposito <eesposit@redhat.com>

The only difference is that blk_ checks if the block is available,
but this check is already performed above in blk_check_byte_request().

This is in preparation for the graph rdlock, which will be taken
by both the callers of blk_check_byte_request() and blk_getlength().

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/block-backend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Vladimir Sementsov-Ogievskiy Dec. 16, 2022, 5:22 p.m. UTC | #1
On 12/13/22 11:53, Kevin Wolf wrote:
> From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> 
> The only difference is that blk_ checks if the block is available,
> but this check is already performed above in blk_check_byte_request().
> 
> This is in preparation for the graph rdlock, which will be taken
> by both the callers of blk_check_byte_request() and blk_getlength().
> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block/block-backend.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 0194d86113..5b8da86772 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -1253,7 +1253,7 @@ static int blk_check_byte_request(BlockBackend *blk, int64_t offset,
>       }
>   
>       if (!blk->allow_write_beyond_eof) {
> -        len = blk_getlength(blk);
> +        len = bdrv_getlength(blk_bs(blk));

I understand the reasoning, but the change looks like a degradation.. bdrv_* functions becomes kind of *_locked() ? If we are going to introduce a lot of such changes, that's not good. But this one is not a problem of course.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

>           if (len < 0) {
>               return len;
>           }
Emanuele Giuseppe Esposito Dec. 19, 2022, 12:28 p.m. UTC | #2
Am 16/12/2022 um 18:22 schrieb Vladimir Sementsov-Ogievskiy:
> On 12/13/22 11:53, Kevin Wolf wrote:
>> From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>>
>> The only difference is that blk_ checks if the block is available,
>> but this check is already performed above in blk_check_byte_request().
>>
>> This is in preparation for the graph rdlock, which will be taken
>> by both the callers of blk_check_byte_request() and blk_getlength().
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>   block/block-backend.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/block/block-backend.c b/block/block-backend.c
>> index 0194d86113..5b8da86772 100644
>> --- a/block/block-backend.c
>> +++ b/block/block-backend.c
>> @@ -1253,7 +1253,7 @@ static int blk_check_byte_request(BlockBackend
>> *blk, int64_t offset,
>>       }
>>         if (!blk->allow_write_beyond_eof) {
>> -        len = blk_getlength(blk);
>> +        len = bdrv_getlength(blk_bs(blk));
> 
> I understand the reasoning, but the change looks like a degradation..
> bdrv_* functions becomes kind of *_locked() ? If we are going to
> introduce a lot of such changes, that's not good. But this one is not a
> problem of course.

Nope, that's the only one (you can check my previous series "part 1",
"part 2" and "part 3" to have an idea on what is coming).

Thank you,
Emanuele
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> 
>>           if (len < 0) {
>>               return len;
>>           }
>
diff mbox series

Patch

diff --git a/block/block-backend.c b/block/block-backend.c
index 0194d86113..5b8da86772 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1253,7 +1253,7 @@  static int blk_check_byte_request(BlockBackend *blk, int64_t offset,
     }
 
     if (!blk->allow_write_beyond_eof) {
-        len = blk_getlength(blk);
+        len = bdrv_getlength(blk_bs(blk));
         if (len < 0) {
             return len;
         }