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;
+    }
 
     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));
-- 
1.8.3.1

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));
> >
>
>
>