On Fri, 2019-07-05 at 13:58 +0200, Max Reitz wrote: > 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 <mlevi...@redhat.com> > > --- > > 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/ Oops.
> > > + 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 Of course, thanks!! > > > + 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 */ > > Thanks for the review, Best regards, Maxim Levitsky