Re: [PATCH v5 09/13] hw/block/nvme: parameterize nvme_ns_nlbas

[Minwoo Im]

On 21-03-16 08:19:08, Klaus Jensen wrote:
> On Mar 16 15:53, Minwoo Im wrote:
> > On 21-03-10 10:53:43, Klaus Jensen wrote:
> > > From: Klaus Jensen <k.jensen@samsung.com>
> > > 
> > > Provide a more flexible nlbas helper.
> > > 
> > > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> > > ---
> > >  hw/block/nvme-ns.h | 14 ++++++++++----
> > >  1 file changed, 10 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h
> > > index 07e16880801d..34f9474a1cd1 100644
> > > --- a/hw/block/nvme-ns.h
> > > +++ b/hw/block/nvme-ns.h
> > > @@ -136,12 +136,18 @@ static inline bool nvme_ns_ext(NvmeNamespace *ns)
> > >  }
> > >  
> > >  /* calculate the number of LBAs that the namespace can accomodate */
> > > +static inline uint64_t __nvme_nlbas(size_t size, uint8_t lbads, uint16_t 
> > > ms)
> > > +{
> > > +    if (ms) {
> > > +        return size / ((1 << lbads) + ms);
> > > +    }
> > > +
> > > +    return size >> lbads;
> > > +}
> > > +
> > >  static inline uint64_t nvme_ns_nlbas(NvmeNamespace *ns)
> > >  {
> > > -    if (nvme_msize(ns)) {
> > > -        return ns->size / (nvme_lsize(ns) + nvme_msize(ns));
> > > -    }
> > > -    return ns->size >> nvme_ns_lbads(ns);
> > > +    return __nvme_nlbas(ns->size, nvme_ns_lbads(ns), nvme_msize(ns));
> > >  }
> > 
> > Hmm.. I think it looks like __nvme_nlbas does the same with the
> > nvme_ns_nlbas, but flexible argument attributes.  But I think those
> > three attributes are all for ns-specific fields which is not that
> > generic so that I don't think we are going to take the helper from much
> > more general perspective with __nvme_nlbas.
> > 
> 
> This patch should be moved two patches forward in the series - it is
> used in [12/13] to check the zone geometry before the values are set on
> the namespace proper. This is also used in Format NVM to verify the
> format before formatting ("commiting" the values on the NvmeNamespace
> structure).

Checked [12/13] right before.  Thanks for pointing that out!

Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>