Re: [Qemu-block] [PATCH v3 3/6] block/nvme: support larger that 512 bytes sector devices

[Max Reitz]

On 03.07.19 17:59, Maxim Levitsky wrote:
> Currently the driver hardcodes the sector size to 512,
> and doesn't check the underlying device. Fix that.
> 
> Also fail if underlying nvme device is formatted with metadata
> as this needs special support.
> 
> Signed-off-by: Maxim Levitsky <address@hidden>
> ---
>  block/nvme.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 52798081b2..1f0d09349f 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c

[...]

> @@ -463,7 +467,22 @@ static void nvme_identify(BlockDriverState *bs, int 
> namespace, Error **errp)
>      }
>  
>      s->nsze = le64_to_cpu(idns->nsze);
> +    lbaf = &idns->lbaf[NVME_ID_NS_FLBAS_INDEX(idns->flbas)];
> +
> +    if (lbaf->ms) {
> +        error_setg(errp, "Namespaces with metadata are not yet supported");
> +        goto out;
> +    }
> +
> +    hwsect_size = 1 << lbaf->ds;
> +
> +    if (hwsect_size < BDRV_SECTOR_BITS || hwsect_size > s->page_size) {

s/BDRV_SECTOR_BITS/BDRV_SECTOR_SIZE/

> +        error_setg(errp, "Namespace has unsupported block size (%d)",
> +                hwsect_size);
> +        goto out;
> +    }
>  
> +    s->blkshift = lbaf->ds;
>  out:
>      qemu_vfio_dma_unmap(s->vfio, resp);
>      qemu_vfree(resp);
> @@ -782,8 +801,22 @@ fail:
>  static int64_t nvme_getlength(BlockDriverState *bs)
>  {
>      BDRVNVMeState *s = bs->opaque;
> +    return s->nsze << s->blkshift;
> +}
>  
> -    return s->nsze << BDRV_SECTOR_BITS;
> +static int64_t nvme_get_blocksize(BlockDriverState *bs)
> +{
> +    BDRVNVMeState *s = bs->opaque;
> +    assert(s->blkshift >= 9);

I think BDRV_SECTOR_BITS is more correct here (this is about what the
general block layer code expects).  Also, there’s no pain in doing so,
as you did check against BDRV_SECTOR_SIZE in nvme_identify().

Max

> +    return 1 << s->blkshift;
> +}
> +
> +static int nvme_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
> +{
> +    int64_t blocksize = nvme_get_blocksize(bs);
> +    bsz->phys = blocksize;
> +    bsz->log = blocksize;
> +    return 0;
>  }
>  
>  /* Called with s->dma_map_lock */

signature.asc
Description: OpenPGP digital signature