[v8,02/21] nvme: Drop pointless .bdrv_co_get_block_status()

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

Commit Message

Eric Blake Feb. 13, 2018, 8:26 p.m.
Commit bdd6a90 has a bug: drivers should never directly set
BDRV_BLOCK_ALLOCATED, but only io.c should do that (as needed).
Instead, drivers should report BDRV_BLOCK_DATA if it knows that
data comes from this BDS.

But let's look at the bigger picture: semantically, the nvme
driver is similar to the nbd, null, and raw drivers (no backing
file, all data comes from this BDS).  But while two of those
other drivers have to supply the callback (null because it can
special-case BDRV_BLOCK_ZERO, raw because it can special-case
a different offset), in this case the block layer defaults are
good enough without the callback at all (similar to nbd).

So, fix the bug by deletion ;)

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

---
v8: new patch
---
 block/nvme.c | 14 --------------
 1 file changed, 14 deletions(-)

Comments

Philippe Mathieu-Daudé Feb. 14, 2018, 5:41 p.m. | #1
On 02/13/2018 05:26 PM, Eric Blake wrote:
> Commit bdd6a90 has a bug: drivers should never directly set
> BDRV_BLOCK_ALLOCATED, but only io.c should do that (as needed).

Doesn't "pointless" in subject hide this is a bugfix?

> Instead, drivers should report BDRV_BLOCK_DATA if it knows that
> data comes from this BDS.
> 
> But let's look at the bigger picture: semantically, the nvme
> driver is similar to the nbd, null, and raw drivers (no backing
> file, all data comes from this BDS).  But while two of those
> other drivers have to supply the callback (null because it can
> special-case BDRV_BLOCK_ZERO, raw because it can special-case
> a different offset), in this case the block layer defaults are
> good enough without the callback at all (similar to nbd).
> 
> So, fix the bug by deletion ;)
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> ---
> v8: new patch
> ---
>  block/nvme.c | 14 --------------
>  1 file changed, 14 deletions(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 10bffbbf2f4..4e561b08df3 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -1068,18 +1068,6 @@ static int nvme_reopen_prepare(BDRVReopenState *reopen_state,
>      return 0;
>  }
> 
> -static int64_t coroutine_fn nvme_co_get_block_status(BlockDriverState *bs,
> -                                                     int64_t sector_num,
> -                                                     int nb_sectors, int *pnum,
> -                                                     BlockDriverState **file)
> -{
> -    *pnum = nb_sectors;
> -    *file = bs;
> -
> -    return BDRV_BLOCK_ALLOCATED | BDRV_BLOCK_OFFSET_VALID |
> -           (sector_num << BDRV_SECTOR_BITS);
> -}
> -
>  static void nvme_refresh_filename(BlockDriverState *bs, QDict *opts)
>  {
>      QINCREF(opts);
> @@ -1179,8 +1167,6 @@ static BlockDriver bdrv_nvme = {
>      .bdrv_co_flush_to_disk    = nvme_co_flush,
>      .bdrv_reopen_prepare      = nvme_reopen_prepare,
> 
> -    .bdrv_co_get_block_status = nvme_co_get_block_status,
> -
>      .bdrv_refresh_filename    = nvme_refresh_filename,
>      .bdrv_refresh_limits      = nvme_refresh_limits,
>

Patch

diff --git a/block/nvme.c b/block/nvme.c
index 10bffbbf2f4..4e561b08df3 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1068,18 +1068,6 @@  static int nvme_reopen_prepare(BDRVReopenState *reopen_state,
     return 0;
 }

-static int64_t coroutine_fn nvme_co_get_block_status(BlockDriverState *bs,
-                                                     int64_t sector_num,
-                                                     int nb_sectors, int *pnum,
-                                                     BlockDriverState **file)
-{
-    *pnum = nb_sectors;
-    *file = bs;
-
-    return BDRV_BLOCK_ALLOCATED | BDRV_BLOCK_OFFSET_VALID |
-           (sector_num << BDRV_SECTOR_BITS);
-}
-
 static void nvme_refresh_filename(BlockDriverState *bs, QDict *opts)
 {
     QINCREF(opts);
@@ -1179,8 +1167,6 @@  static BlockDriver bdrv_nvme = {
     .bdrv_co_flush_to_disk    = nvme_co_flush,
     .bdrv_reopen_prepare      = nvme_reopen_prepare,

-    .bdrv_co_get_block_status = nvme_co_get_block_status,
-
     .bdrv_refresh_filename    = nvme_refresh_filename,
     .bdrv_refresh_limits      = nvme_refresh_limits,