Re: [Qemu-devel] [PATCH v9 03/11] pci: Convert msix_init() to Error and fix callers

[Hannes Reinecke]

On 01/17/2017 07:18 AM, Cao jin wrote:
> msix_init() reports errors with error_report(), which is wrong when
> it's used in realize().  The same issue was fixed for msi_init() in
> commit 1108b2f. In order to make the API change as small as possible,
> leave the return value check to later patch.
> 
> For some devices(like e1000e, vmxnet3, nvme) who won't fail because of
> msix_init's failure, suppress the error report by passing NULL error
> object.
> 
> Bonus: add comment for msix_init.
> 
> CC: Jiri Pirko 
> CC: Gerd Hoffmann 
> CC: Dmitry Fleytman 
> CC: Jason Wang 
> CC: Michael S. Tsirkin 
> CC: Hannes Reinecke 
> CC: Paolo Bonzini 
> CC: Alex Williamson 
> CC: Markus Armbruster 
> CC: Marcel Apfelbaum 
> Signed-off-by: Cao jin 
> ---
>  hw/block/nvme.c|  2 +-
>  hw/misc/ivshmem.c  |  8 
>  hw/net/e1000e.c|  2 +-
>  hw/net/rocker/rocker.c |  4 +++-
>  hw/net/vmxnet3.c   |  2 +-
>  hw/pci/msix.c  | 36 +++-
>  hw/scsi/megasas.c  |  4 +++-
>  hw/usb/hcd-xhci.c  |  4 ++--
>  hw/vfio/pci.c  |  8 ++--
>  hw/virtio/virtio-pci.c |  4 ++--
>  include/hw/pci/msix.h  |  5 +++--
>  11 files changed, 57 insertions(+), 22 deletions(-)
> 
For megasas: Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[Qemu-devel] [PATCH v9 03/11] pci: Convert msix_init() to Error and fix callers

[Cao jin]

msix_init() reports errors with error_report(), which is wrong when
it's used in realize().  The same issue was fixed for msi_init() in
commit 1108b2f. In order to make the API change as small as possible,
leave the return value check to later patch.

For some devices(like e1000e, vmxnet3, nvme) who won't fail because of
msix_init's failure, suppress the error report by passing NULL error
object.

Bonus: add comment for msix_init.

CC: Jiri Pirko 
CC: Gerd Hoffmann 
CC: Dmitry Fleytman 
CC: Jason Wang 
CC: Michael S. Tsirkin 
CC: Hannes Reinecke 
CC: Paolo Bonzini 
CC: Alex Williamson 
CC: Markus Armbruster 
CC: Marcel Apfelbaum 
Signed-off-by: Cao jin 
---
 hw/block/nvme.c|  2 +-
 hw/misc/ivshmem.c  |  8 
 hw/net/e1000e.c|  2 +-
 hw/net/rocker/rocker.c |  4 +++-
 hw/net/vmxnet3.c   |  2 +-
 hw/pci/msix.c  | 36 +++-
 hw/scsi/megasas.c  |  4 +++-
 hw/usb/hcd-xhci.c  |  4 ++--
 hw/vfio/pci.c  |  8 ++--
 hw/virtio/virtio-pci.c |  4 ++--
 include/hw/pci/msix.h  |  5 +++--
 11 files changed, 57 insertions(+), 22 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index d479fd22f573..ae91a18f1724 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -872,7 +872,7 @@ static int nvme_init(PCIDevice *pci_dev)
 pci_register_bar(>parent_obj, 0,
 PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64,
 >iomem);
-msix_init_exclusive_bar(>parent_obj, n->num_queues, 4);
+msix_init_exclusive_bar(>parent_obj, n->num_queues, 4, NULL);
 
 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/misc/ivshmem.c b/hw/misc/ivshmem.c
index abeaf3da0800..70e71a597b9c 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -749,13 +749,13 @@ static void ivshmem_reset(DeviceState *d)
 }
 }
 
-static int ivshmem_setup_interrupts(IVShmemState *s)
+static int ivshmem_setup_interrupts(IVShmemState *s, Error **errp)
 {
 /* allocate QEMU callback data for receiving interrupts */
 s->msi_vectors = g_malloc0(s->vectors * sizeof(MSIVector));
 
 if (ivshmem_has_feature(s, IVSHMEM_MSI)) {
-if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1)) {
+if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1, errp)) {
 return -1;
 }
 
@@ -897,8 +897,8 @@ static void ivshmem_common_realize(PCIDevice *dev, Error 
**errp)
 qemu_chr_fe_set_handlers(>server_chr, ivshmem_can_receive,
  ivshmem_read, NULL, s, NULL, true);
 
-if (ivshmem_setup_interrupts(s) < 0) {
-error_setg(errp, "failed to initialize interrupts");
+if (ivshmem_setup_interrupts(s, errp) < 0) {
+error_prepend(errp, "Failed to initialize interrupts: ");
 return;
 }
 }
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 4994e1ca0062..ed04adce061c 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -292,7 +292,7 @@ e1000e_init_msix(E1000EState *s)
 E1000E_MSIX_IDX, E1000E_MSIX_TABLE,
 >msix,
 E1000E_MSIX_IDX, E1000E_MSIX_PBA,
-0xA0);
+0xA0, NULL);
 
 if (res < 0) {
 trace_e1000e_msix_init_fail(res);
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index e9d215aa4df1..6e70fddee36b 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -1256,14 +1256,16 @@ static int rocker_msix_init(Rocker *r)
 {
 PCIDevice *dev = PCI_DEVICE(r);
 int err;
+Error *local_err = NULL;
 
 err = msix_init(dev, ROCKER_MSIX_VEC_COUNT(r->fp_ports),
 >msix_bar,
 ROCKER_PCI_MSIX_BAR_IDX, ROCKER_PCI_MSIX_TABLE_OFFSET,
 >msix_bar,
 ROCKER_PCI_MSIX_BAR_IDX, ROCKER_PCI_MSIX_PBA_OFFSET,
-0);
+0, _err);
 if (err) {
+error_report_err(local_err);
 return err;
 }
 
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 92f6af9620f1..7b2971fe5902 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2191,7 +2191,7 @@ vmxnet3_init_msix(VMXNET3State *s)
 VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_TABLE,
 >msix_bar,
 VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_PBA(s),
-VMXNET3_MSIX_OFFSET(s));
+VMXNET3_MSIX_OFFSET(s), NULL);
 
 if (0 > res) {
 VMW_WRPRN("Failed to initialize MSI-X, error %d", res);
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 0cee631ecc55..bd8cdd41d880 100644
---