Re: [Qemu-block] [Qemu-devel] [PATCH v8 02/21] nvme: Drop pointless .bdrv_co_get_block_status()

[Philippe Mathieu-Daudé]

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 <address@hidden>
> 
> ---
> 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,
>