Re: [PATCH v6 10/42] nvme: refactor device realization

[Klaus Birkelund Jensen]

On Mar 25 12:40, Maxim Levitsky wrote:
> On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote:
> > From: Klaus Jensen <address@hidden>
> > 
> > This patch splits up nvme_realize into multiple individual functions,
> > each initializing a different subset of the device.
> > 
> > Signed-off-by: Klaus Jensen <address@hidden>
> > Acked-by: Keith Busch <address@hidden>
> > ---
> >  hw/block/nvme.c | 178 ++++++++++++++++++++++++++++++------------------
> >  hw/block/nvme.h |  23 ++++++-
> >  2 files changed, 134 insertions(+), 67 deletions(-)
> > 
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index 7dfd8a1a392d..665485045066 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -1340,57 +1337,100 @@ static void nvme_realize(PCIDevice *pci_dev, Error 
> > **errp)
> >          n->params.max_ioqpairs = n->params.num_queues - 1;
> >      }
> >  
> > -    if (!n->params.max_ioqpairs) {
> > -        error_setg(errp, "max_ioqpairs can't be less than 1");
> > +    if (params->max_ioqpairs < 1 ||
> > +        params->max_ioqpairs > PCI_MSIX_FLAGS_QSIZE) {
> > +        error_setg(errp, "nvme: max_ioqpairs must be ");
> Looks like the error message is not complete now.

Fixed!

> 
> Small nitpick: To be honest this not only refactoring in the device 
> realization since you also (rightfully)
> removed the duplicated cmbsz/cmbloc so I would add a mention for this in the 
> commit message.
> But that doesn't matter that much, so
> 

You are right. I've added it as a separate patch.

> Reviewed-by: Maxim Levitsky <address@hidden>
> 
> Best regards,
>       Maxim Levitsky
>