| Message ID | 1540876720-9574-4-git-send-email-liq3ea@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | nvme: some small fixes | expand |
On 30.10.18 06:18, Li Qiang wrote: > As this function can fail. > > Signed-off-by: Li Qiang <liq3ea@gmail.com> > --- > hw/block/nvme.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 72c9644..a406c23 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -1250,7 +1250,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) > pci_register_bar(&n->parent_obj, 0, > PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64, > &n->iomem); > - msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, NULL); > + if (msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, NULL)) { > + error_setg(errp, "msix_init_exclusive_bar failed"); > + return; > + } Commit ee640c625e1 which last touched this function stated that creating this device won't fail if this function fails, so I suppose it's intential that the error isn't checked. But if you think the error should be checked and device creation should be aborted if this function failed, then I suppose there'd be some cleaning up to instead of just returning. Also, we should just pass errp to that function (as its last parameter, instead of NULL). Max > > id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID)); > id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID)); >
Max Reitz <mreitz@redhat.com> 于2019年1月9日周三 下午10:52写道： > On 30.10.18 06:18, Li Qiang wrote: > > As this function can fail. > > > > Signed-off-by: Li Qiang <liq3ea@gmail.com> > > --- > > hw/block/nvme.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > > index 72c9644..a406c23 100644 > > --- a/hw/block/nvme.c > > +++ b/hw/block/nvme.c > > @@ -1250,7 +1250,10 @@ static void nvme_realize(PCIDevice *pci_dev, > Error **errp) > > pci_register_bar(&n->parent_obj, 0, > > PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64, > > &n->iomem); > > - msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, NULL); > > + if (msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, > NULL)) { > > + error_setg(errp, "msix_init_exclusive_bar failed"); > > + return; > > + } > > Commit ee640c625e1 which last touched this function stated that creating > this device won't fail if this function fails, so I suppose it's > intential that the error isn't checked. > OK, got. I will drop this patch later. Thanks, Li Qiang > > > But if you think the error should be checked and device creation should > be aborted if this function failed, then I suppose there'd be some > cleaning up to instead of just returning. > > Also, we should just pass errp to that function (as its last parameter, > instead of NULL). > > Max > > > > > id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID)); > > id->ssvid = cpu_to_le16(pci_get_word(pci_conf + > PCI_SUBSYSTEM_VENDOR_ID)); > > > > >
diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 72c9644..a406c23 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -1250,7 +1250,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) pci_register_bar(&n->parent_obj, 0, PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64, &n->iomem); - msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, NULL); + if (msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, NULL)) { + error_setg(errp, "msix_init_exclusive_bar failed"); + return; + } id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID)); id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
As this function can fail. Signed-off-by: Li Qiang <liq3ea@gmail.com> --- hw/block/nvme.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)