[v4,19/20] vvfat: Switch to .bdrv_co_block_status()

Message ID 20171012185916.22776-20-eblake@redhat.com
State New
Headers show
Series
  • add byte-based block_status driver callbacks
Related show

Commit Message

Eric Blake Oct. 12, 2017, 6:59 p.m.
We are gradually moving away from sector-based interfaces, towards
byte-based.  Update the vvfat driver accordingly.  Note that we
can rely on the block driver having already clamped limits to our
block size, and simplify accordingly.

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v4: rebase to interface tweak
v3: no change
v2: rebase to earlier changes, simplify
---
 block/vvfat.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

Comments

Vladimir Sementsov-Ogievskiy Nov. 30, 2017, 12:25 p.m. | #1
12.10.2017 21:59, Eric Blake wrote:
> We are gradually moving away from sector-based interfaces, towards
> byte-based.  Update the vvfat driver accordingly.  Note that we
> can rely on the block driver having already clamped limits to our
> block size, and simplify accordingly.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

>
> ---
> v4: rebase to interface tweak
> v3: no change
> v2: rebase to earlier changes, simplify
> ---
>   block/vvfat.c | 16 +++++++---------
>   1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/block/vvfat.c b/block/vvfat.c
> index a0f2335894..9142117fc6 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -3082,15 +3082,13 @@ vvfat_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
>       return ret;
>   }
>
> -static int64_t coroutine_fn vvfat_co_get_block_status(BlockDriverState *bs,
> -        int64_t sector_num, int nb_sectors, int *n, BlockDriverState **file)
> +static int coroutine_fn vvfat_co_block_status(BlockDriverState *bs,
> +                                              bool want_zero, int64_t offset,
> +                                              int64_t bytes, int64_t *n,

may be rename to *pnum ?

> +                                              int64_t *map,
> +                                              BlockDriverState **file)
>   {
> -    *n = bs->total_sectors - sector_num;
> -    if (*n > nb_sectors) {
> -        *n = nb_sectors;
> -    } else if (*n < 0) {
> -        return 0;
> -    }
> +    *n = bytes;
>       return BDRV_BLOCK_DATA;
>   }
>
> @@ -3251,7 +3249,7 @@ static BlockDriver bdrv_vvfat = {
>
>       .bdrv_co_preadv         = vvfat_co_preadv,
>       .bdrv_co_pwritev        = vvfat_co_pwritev,
> -    .bdrv_co_get_block_status = vvfat_co_get_block_status,
> +    .bdrv_co_block_status   = vvfat_co_block_status,
>   };
>
>   static void bdrv_vvfat_init(void)
Eric Blake Nov. 30, 2017, 11:31 p.m. | #2
On 11/30/2017 06:25 AM, Vladimir Sementsov-Ogievskiy wrote:
> 12.10.2017 21:59, Eric Blake wrote:
>> We are gradually moving away from sector-based interfaces, towards
>> byte-based.  Update the vvfat driver accordingly.  Note that we
>> can rely on the block driver having already clamped limits to our
>> block size, and simplify accordingly.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 

>> vvfat_co_get_block_status(BlockDriverState *bs,
>> -        int64_t sector_num, int nb_sectors, int *n, BlockDriverState 
>> **file)
>> +static int coroutine_fn vvfat_co_block_status(BlockDriverState *bs,
>> +                                              bool want_zero, int64_t 
>> offset,
>> +                                              int64_t bytes, int64_t *n,
> 
> may be rename to *pnum ?

I don't see it making a difference; it would match the callback 
definition more closely, but the code is short enough that it's not that 
confusing with keeping the naming as-is.

Patch

diff --git a/block/vvfat.c b/block/vvfat.c
index a0f2335894..9142117fc6 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3082,15 +3082,13 @@  vvfat_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
     return ret;
 }

-static int64_t coroutine_fn vvfat_co_get_block_status(BlockDriverState *bs,
-        int64_t sector_num, int nb_sectors, int *n, BlockDriverState **file)
+static int coroutine_fn vvfat_co_block_status(BlockDriverState *bs,
+                                              bool want_zero, int64_t offset,
+                                              int64_t bytes, int64_t *n,
+                                              int64_t *map,
+                                              BlockDriverState **file)
 {
-    *n = bs->total_sectors - sector_num;
-    if (*n > nb_sectors) {
-        *n = nb_sectors;
-    } else if (*n < 0) {
-        return 0;
-    }
+    *n = bytes;
     return BDRV_BLOCK_DATA;
 }

@@ -3251,7 +3249,7 @@  static BlockDriver bdrv_vvfat = {

     .bdrv_co_preadv         = vvfat_co_preadv,
     .bdrv_co_pwritev        = vvfat_co_pwritev,
-    .bdrv_co_get_block_status = vvfat_co_get_block_status,
+    .bdrv_co_block_status   = vvfat_co_block_status,
 };

 static void bdrv_vvfat_init(void)