On 21-01-21 15:17:16, Keith Busch wrote: > On Fri, Jan 22, 2021 at 07:09:06AM +0900, Minwoo Im wrote: > > -static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev) > > +static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev, uint16_t > > cntlid) > > { > > NvmeIdCtrl *id = &n->id_ctrl; > > uint8_t *pci_conf = pci_dev->config; > > > > + n->cntlid = cntlid; > > I don't think 'cntlid' is important enough to be a function parameter. > You can just set it within the 'NvmeCtrl' struct before calling this > function like all the other properties.
Okay. Rather than adding a parameter to this function, nvme_init_subsys() may take this job to assign cntlid to the controller instance first. Let me fix one! > > @@ -4517,7 +4543,11 @@ static void nvme_realize(PCIDevice *pci_dev, Error > > **errp) > > return; > > } > > > > - nvme_init_ctrl(n, pci_dev); > > + cntlid = nvme_init_subsys(n, errp); > > + if (cntlid < 0) { > > error_propogate(); Thanks for catching this.