[PATCH v2 0/4] Fix error handling for callers of load_image_targphys, get_image_size, event_notifier_init, msix_init

[PATCH v2 0/4] Fix error handling for callers of load_image_targphys, get_image_size, event_notifier_init, msix_init

[Trieu Huynh]

From: Trieu Huynh <vikingtc4@gmail.com>

Changes in v2:
- Patch 1: Remove redundant if (ret < 0) checks added after calls that
already use &error_fatal (dead code, pointed out by BALATON Zoltan
and Aditya Gupta). Use &error_fatal directly for NULL callers where
failure is fatal.
- Patch 2: Fix to only exit when size < 0 since it is not mandatory,
either BIOS or FRU (pointed out by Clément Chigot).
- Patch 3: No code changes. Add Acked-by and Reviewed-by tags.
- Patch 4: Use errp param instead of passing NULL as default to handle
failure (pointed out by Akihiko Odaki).

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413

Trieu Huynh (4):
  hw/core/loader: fix error handling for load_image_targphys callers
  hw/core/loader: fix error handling for get_image_size callers
  util/event_notifier: fix error handling for event_notifier_init
    callers
  hw/pci/msix: fix error handling for msix_init callers

 hw/alpha/dp264.c       |  2 +-
 hw/hppa/machine.c      |  2 +-
 hw/hyperv/hyperv.c     |  4 ++--
 hw/hyperv/vmbus.c      |  4 ++--
 hw/ipmi/ipmi_bmc_sim.c |  2 ++
 hw/m68k/next-cube.c    |  2 +-
 hw/m68k/q800.c         |  2 +-
 hw/m68k/virt.c         |  2 +-
 hw/microblaze/boot.c   |  3 ++-
 hw/net/igbvf.c         |  2 +-
 hw/net/rocker/rocker.c |  2 +-
 hw/pci/msix.c          |  2 +-
 hw/remote/proxy.c      | 15 +++++++++++++--
 hw/scsi/megasas.c      | 18 +++++++++++++-----
 hw/sparc/leon3.c       |  4 ++++
 hw/usb/hcd-xhci-pci.c  | 19 ++++++++++++++-----
 hw/vfio/ap.c           |  2 +-
 hw/vfio/ccw.c          |  2 +-
 hw/vfio/pci-quirks.c   |  2 +-
 hw/vfio/pci.c          |  2 +-
 hw/virtio/vhost-vdpa.c |  4 ++--
 21 files changed, 66 insertions(+), 31 deletions(-)

-- 
2.43.0

[PATCH v2 1/4] hw/core/loader: fix error handling for load_image_targphys callers

[Trieu Huynh]

From: Trieu Huynh <vikingtc4@gmail.com>

Use QEMU's Error API to handle load_image_targphys() failures
consistently across callers.

- Use &error_fatal for callers that previously passed NULL, ensuring
the process exits early on failure instead of continuing in an invalid
state.
- No functional changes.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>

---
v2:
- Use &error_fatal instead of manual return value checks.
- Remove redundant checks for ppc callers that already use
&error_fatal.
- Note: This replaces the "ret < 0" approach proposed in v1.
---
 hw/alpha/dp264.c     | 2 +-
 hw/hppa/machine.c    | 2 +-
 hw/m68k/next-cube.c  | 2 +-
 hw/m68k/q800.c       | 2 +-
 hw/m68k/virt.c       | 2 +-
 hw/microblaze/boot.c | 3 ++-
 6 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 98219f0456..2ab3c14747 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -190,7 +190,7 @@ static void clipper_init(MachineState *machine)
             /* Put the initrd image as high in memory as possible.  */
             initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
             load_image_targphys(initrd_filename, initrd_base,
-                                ram_size - initrd_base, NULL);
+                                ram_size - initrd_base, &error_fatal);
 
             address_space_stq_le(&address_space_memory, param_offset + 0x100,
                                  initrd_base + 0xfffffc0000000000ULL,
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index ec63dc1297..99a4c22c73 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -507,7 +507,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
             }
 
             load_image_targphys(initrd_filename, initrd_base, initrd_size,
-                                NULL);
+                                &error_fatal);
             cpu[0]->env.initrd_base = initrd_base;
             cpu[0]->env.initrd_end  = initrd_base + initrd_size;
         }
diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
index 26177c7b86..4bc8e72f3e 100644
--- a/hw/m68k/next-cube.c
+++ b/hw/m68k/next-cube.c
@@ -1326,7 +1326,7 @@ static void next_cube_init(MachineState *machine)
     memory_region_init_alias(&m->rom2, NULL, "next.rom2", &m->rom, 0x0,
                              0x20000);
     memory_region_add_subregion(sysmem, 0x0, &m->rom2);
-    if (load_image_targphys(bios_name, 0x01000000, 0x20000, NULL) < 8) {
+    if (load_image_targphys(bios_name, 0x01000000, 0x20000, &error_fatal) < 8) {
         if (!qtest_enabled()) {
             error_report("Failed to load firmware '%s'.", bios_name);
         }
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index ded531394e..c0d78eb7d7 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -633,7 +633,7 @@ static void q800_machine_init(MachineState *machine)
 
             initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
             load_image_targphys(initrd_filename, initrd_base,
-                                ram_size - initrd_base, NULL);
+                                ram_size - initrd_base, &error_fatal);
             BOOTINFO2(param_ptr, BI_RAMDISK, initrd_base,
                       initrd_size);
         } else {
diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c
index e67900c727..ffe6e23415 100644
--- a/hw/m68k/virt.c
+++ b/hw/m68k/virt.c
@@ -292,7 +292,7 @@ static void virt_init(MachineState *machine)
 
             initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
             load_image_targphys(initrd_filename, initrd_base,
-                                ram_size - initrd_base, NULL);
+                                ram_size - initrd_base, &error_fatal);
             BOOTINFO2(param_ptr, BI_RAMDISK, initrd_base,
                       initrd_size);
         } else {
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index a6f9ebab90..4ad5ffd34b 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -38,6 +38,7 @@
 #include "hw/core/loader.h"
 #include "elf.h"
 #include "qemu/cutils.h"
+#include "qapi/error.h"
 
 #include "boot.h"
 
@@ -171,7 +172,7 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu, bool is_little_endian,
         /* Not an ELF image nor an u-boot image, try a RAW image.  */
         if (kernel_size < 0) {
             kernel_size = load_image_targphys(kernel_filename, ddr_base,
-                                              ramsize, NULL);
+                                              ramsize, &error_fatal);
             boot_info.bootstrap_pc = ddr_base;
             high = (ddr_base + kernel_size + 3) & ~3;
         }
-- 
2.43.0

[PATCH v2 2/4] hw/core/loader: fix error handling for get_image_size callers

[Trieu Huynh]

From: Trieu Huynh <vikingtc4@gmail.com>

Check the return value of get_image_size() and report failures
for non-mandatory files such as BIOS or FRU images.

- Use ret < 0 to detect failures in getting image size.
- No functional changes.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>

---
v2:
- Keep error reporting as it is.
- Remove exit on error since it is not mandatory.
---
 hw/ipmi/ipmi_bmc_sim.c | 2 ++
 hw/sparc/leon3.c       | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 012e2ee4fe..fd875491f5 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -2561,6 +2561,8 @@ static void ipmi_fru_init(IPMIFru *fru)
             g_free(fru->data);
             fru->data = NULL;
         }
+    } else {
+        error_report("Could not get file size '%s'", fru->filename);
     }
 
 out:
diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 9147d56bd4..122366829d 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -354,6 +354,10 @@ static void leon3_generic_hw_init(MachineState *machine)
     } else {
         bios_size = -1;
     }
+    if (bios_size < 0) {
+        error_report("could not found or failed to get file size: '%s'",
+                     filename);
+    }
 
     if (bios_size > prom_size) {
         error_report("could not load prom '%s': file too big", filename);
-- 
2.43.0

[PATCH v2 3/4] util/event_notifier: fix error handling for event_notifier_init callers

[Trieu Huynh]

From: Trieu Huynh <vikingtc4@gmail.com>

Check return value of event_notifier_init() and return early on
failure instead of continuing with invalid state.
- Use ret < 0 to handle negative return value.
- No functional changes.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
Acked-by: Anthony Krowiak <akrowiak@linux.ibm.com>
Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
---
 hw/hyperv/hyperv.c     |  4 ++--
 hw/hyperv/vmbus.c      |  4 ++--
 hw/remote/proxy.c      | 15 +++++++++++++--
 hw/vfio/ap.c           |  2 +-
 hw/vfio/ccw.c          |  2 +-
 hw/vfio/pci-quirks.c   |  2 +-
 hw/vfio/pci.c          |  2 +-
 hw/virtio/vhost-vdpa.c |  4 ++--
 8 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
index 27e323a819..aa278b179e 100644
--- a/hw/hyperv/hyperv.c
+++ b/hw/hyperv/hyperv.c
@@ -439,7 +439,7 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
         sint_route->staged_msg->cb_data = cb_data;
 
         r = event_notifier_init(ack_notifier, false);
-        if (r) {
+        if (r < 0) {
             goto cleanup_err_sint;
         }
         event_notifier_set_handler(ack_notifier, sint_ack_handler);
@@ -453,7 +453,7 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
 
     /* We need to setup a GSI for this SintRoute */
     r = event_notifier_init(&sint_route->sint_set_notifier, false);
-    if (r) {
+    if (r < 0) {
         goto cleanup_err_sint;
     }
 
diff --git a/hw/hyperv/vmbus.c b/hw/hyperv/vmbus.c
index 64abe4c4c1..5388f4277f 100644
--- a/hw/hyperv/vmbus.c
+++ b/hw/hyperv/vmbus.c
@@ -1432,7 +1432,7 @@ static void open_channel(VMBusChannel *chan)
         goto put_gpadl;
     }
 
-    if (event_notifier_init(&chan->notifier, 0)) {
+    if (event_notifier_init(&chan->notifier, 0) < 0) {
         goto put_gpadl;
     }
 
@@ -2450,7 +2450,7 @@ static void vmbus_realize(BusState *bus, Error **errp)
     }
 
     ret = event_notifier_init(&vmbus->notifier, 0);
-    if (ret != 0) {
+    if (ret < 0) {
         error_setg(errp, "event notifier failed to init with %d", ret);
         goto remove_msg_handler;
     }
diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
index 5081d67e7f..e91566509f 100644
--- a/hw/remote/proxy.c
+++ b/hw/remote/proxy.c
@@ -52,9 +52,20 @@ static void setup_irqfd(PCIProxyDev *dev)
     PCIDevice *pci_dev = PCI_DEVICE(dev);
     MPQemuMsg msg;
     Error *local_err = NULL;
+    int ret = 0;
 
-    event_notifier_init(&dev->intr, 0);
-    event_notifier_init(&dev->resample, 0);
+    ret = event_notifier_init(&dev->intr, 0);
+    if (ret < 0) {
+        error_report("Failed to init intr notifier: %s", strerror(-ret));
+        return;
+    }
+
+    ret = event_notifier_init(&dev->resample, 0);
+    if (ret < 0) {
+        error_report("Failed to init resample notifier: %s", strerror(-ret));
+        event_notifier_cleanup(&dev->intr);
+        return;
+    }
 
     memset(&msg, 0, sizeof(MPQemuMsg));
     msg.cmd = MPQEMU_CMD_SET_IRQFD;
diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index e58a0169af..5c8f305653 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -180,7 +180,7 @@ static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
         return false;
     }
 
-    if (event_notifier_init(notifier, 0)) {
+    if (event_notifier_init(notifier, 0) < 0) {
         error_setg_errno(errp, errno,
                          "vfio: Unable to init event notifier for irq (%d)",
                          irq);
diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 2251facb35..ce9c014e6a 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -418,7 +418,7 @@ static bool vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev,
         return false;
     }
 
-    if (event_notifier_init(notifier, 0)) {
+    if (event_notifier_init(notifier, 0) < 0) {
         error_setg_errno(errp, errno,
                          "vfio: Unable to init event notifier for irq (%d)",
                          irq);
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 7b907b9360..66e02b15a4 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -318,7 +318,7 @@ static VFIOIOEventFD *vfio_ioeventfd_init(VFIOPCIDevice *vdev,
 
     ioeventfd = g_malloc0(sizeof(*ioeventfd));
 
-    if (event_notifier_init(&ioeventfd->e, 0)) {
+    if (event_notifier_init(&ioeventfd->e, 0) < 0) {
         g_free(ioeventfd);
         return NULL;
     }
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 94c174a773..1945751ffd 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -70,7 +70,7 @@ static bool vfio_notifier_init(VFIOPCIDevice *vdev, EventNotifier *e,
     }
 
     ret = event_notifier_init(e, 0);
-    if (ret) {
+    if (ret < 0) {
         error_setg_errno(errp, -ret, "vfio_notifier_init %s failed", name);
         return false;
     }
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 2f8f11df86..9c7634e243 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1075,13 +1075,13 @@ static int vhost_vdpa_svq_set_fds(struct vhost_dev *dev,
     int r;
 
     r = event_notifier_init(&svq->hdev_kick, 0);
-    if (r != 0) {
+    if (r < 0) {
         error_setg_errno(errp, -r, "Couldn't create kick event notifier");
         goto err_init_hdev_kick;
     }
 
     r = event_notifier_init(&svq->hdev_call, 0);
-    if (r != 0) {
+    if (r < 0) {
         error_setg_errno(errp, -r, "Couldn't create call event notifier");
         goto err_init_hdev_call;
     }
-- 
2.43.0

[PATCH v2 4/4] hw/pci/msix: fix error handling for msix_init callers

[Trieu Huynh]

From: Trieu Huynh <vikingtc4@gmail.com>

Check return value of msix_init() and return early on
failure instead of continuing with invalid state.
- Use ret < 0 to handle negative return value.
- Use errp parameter to handle failure instead of NULL.
- No functional changes.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>

---
v2:
- Use errp parameter to handle failure instead of NULL.
---
 hw/net/igbvf.c         |  2 +-
 hw/net/rocker/rocker.c |  2 +-
 hw/pci/msix.c          |  2 +-
 hw/scsi/megasas.c      | 18 +++++++++++++-----
 hw/usb/hcd-xhci-pci.c  | 19 ++++++++++++++-----
 5 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/hw/net/igbvf.c b/hw/net/igbvf.c
index 48d56e43ac..9a165c7063 100644
--- a/hw/net/igbvf.c
+++ b/hw/net/igbvf.c
@@ -260,7 +260,7 @@ static void igbvf_pci_realize(PCIDevice *dev, Error **errp)
 
     ret = msix_init(dev, IGBVF_MSIX_VEC_NUM, &s->msix, IGBVF_MSIX_BAR_IDX, 0,
         &s->msix, IGBVF_MSIX_BAR_IDX, 0x2000, 0x70, errp);
-    if (ret) {
+    if (ret < 0) {
         return;
     }
 
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index 4a7056bd45..910dce901b 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -1228,7 +1228,7 @@ static int rocker_msix_init(Rocker *r, Error **errp)
                     &r->msix_bar,
                     ROCKER_PCI_MSIX_BAR_IDX, ROCKER_PCI_MSIX_PBA_OFFSET,
                     0, errp);
-    if (err) {
+    if (err < 0) {
         return err;
     }
 
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index b35476d057..1b23eaf100 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -432,7 +432,7 @@ int msix_init_exclusive_bar(PCIDevice *dev, uint32_t nentries,
                     0, &dev->msix_exclusive_bar,
                     bar_nr, bar_pba_offset,
                     0, errp);
-    if (ret) {
+    if (ret < 0) {
         return ret;
     }
 
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index f62e420a91..b237ba7656 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2380,11 +2380,19 @@ static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
     memory_region_init_io(&s->queue_io, OBJECT(s), &megasas_queue_ops, s,
                           "megasas-queue", 0x40000);
 
-    if (megasas_use_msix(s) &&
-        msix_init(dev, 15, &s->mmio_io, b->mmio_bar, 0x2000,
-                  &s->mmio_io, b->mmio_bar, 0x3800, 0x68, NULL)) {
-        /* TODO: check msix_init's error, and should fail on msix=on */
-        s->msix = ON_OFF_AUTO_OFF;
+    if (megasas_use_msix(s)) {
+        Error *local_err = NULL;
+        ret = msix_init(dev, 15, &s->mmio_io, b->mmio_bar, 0x2000,
+                        &s->mmio_io, b->mmio_bar, 0x3800, 0x68, &local_err);
+
+        if (ret < 0) {
+            if (s->msix == ON_OFF_AUTO_ON) {
+                error_propagate(errp, local_err);
+                return;
+            }
+            error_free(local_err);
+            s->msix = ON_OFF_AUTO_OFF;
+        }
     }
 
     if (pci_is_express(dev)) {
diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
index aa570506fc..3bdde9b64a 100644
--- a/hw/usb/hcd-xhci-pci.c
+++ b/hw/usb/hcd-xhci-pci.c
@@ -173,11 +173,20 @@ static void usb_xhci_pci_realize(struct PCIDevice *dev, Error **errp)
     }
 
     if (s->msix != ON_OFF_AUTO_OFF) {
-        /* TODO check for errors, and should fail when msix=on */
-        msix_init(dev, s->xhci.numintrs,
-                  &s->xhci.mem, 0, OFF_MSIX_TABLE,
-                  &s->xhci.mem, 0, OFF_MSIX_PBA,
-                  0x90, NULL);
+        Error *local_err = NULL;
+        ret = msix_init(dev, s->xhci.numintrs,
+                        &s->xhci.mem, 0, OFF_MSIX_TABLE,
+                        &s->xhci.mem, 0, OFF_MSIX_PBA,
+                        0x90, &local_err);
+
+        if (ret < 0) {
+            if (s->msix == ON_OFF_AUTO_ON) {
+                error_propagate(errp, local_err);
+                return;
+            }
+            error_free(local_err);
+            s->msix = ON_OFF_AUTO_OFF;
+        }
     }
     s->xhci.as = pci_get_address_space(dev);
 }
-- 
2.43.0

Re: [PATCH v2 3/4] util/event_notifier: fix error handling for event_notifier_init callers

[Cédric Le Goater]

On 3/11/26 19:34, Trieu Huynh wrote:
> From: Trieu Huynh <vikingtc4@gmail.com>
> 
> Check return value of event_notifier_init() and return early on
> failure instead of continuing with invalid state.
> - Use ret < 0 to handle negative return value.
> - No functional changes.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
> Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
> Acked-by: Anthony Krowiak <akrowiak@linux.ibm.com>
> Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
> ---
>   hw/hyperv/hyperv.c     |  4 ++--
>   hw/hyperv/vmbus.c      |  4 ++--
>   hw/remote/proxy.c      | 15 +++++++++++++--
>   hw/vfio/ap.c           |  2 +-
>   hw/vfio/ccw.c          |  2 +-
>   hw/vfio/pci-quirks.c   |  2 +-
>   hw/vfio/pci.c          |  2 +-
>   hw/virtio/vhost-vdpa.c |  4 ++--
>   8 files changed, 23 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
> index 27e323a819..aa278b179e 100644
> --- a/hw/hyperv/hyperv.c
> +++ b/hw/hyperv/hyperv.c
> @@ -439,7 +439,7 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
>           sint_route->staged_msg->cb_data = cb_data;
>   
>           r = event_notifier_init(ack_notifier, false);
> -        if (r) {
> +        if (r < 0) {
>               goto cleanup_err_sint;
>           }
>           event_notifier_set_handler(ack_notifier, sint_ack_handler);
> @@ -453,7 +453,7 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
>   
>       /* We need to setup a GSI for this SintRoute */
>       r = event_notifier_init(&sint_route->sint_set_notifier, false);
> -    if (r) {
> +    if (r < 0) {
>           goto cleanup_err_sint;
>       }
>   
> diff --git a/hw/hyperv/vmbus.c b/hw/hyperv/vmbus.c
> index 64abe4c4c1..5388f4277f 100644
> --- a/hw/hyperv/vmbus.c
> +++ b/hw/hyperv/vmbus.c
> @@ -1432,7 +1432,7 @@ static void open_channel(VMBusChannel *chan)
>           goto put_gpadl;
>       }
>   
> -    if (event_notifier_init(&chan->notifier, 0)) {
> +    if (event_notifier_init(&chan->notifier, 0) < 0) {
>           goto put_gpadl;
>       }
>   
> @@ -2450,7 +2450,7 @@ static void vmbus_realize(BusState *bus, Error **errp)
>       }
>   
>       ret = event_notifier_init(&vmbus->notifier, 0);
> -    if (ret != 0) {
> +    if (ret < 0) {
>           error_setg(errp, "event notifier failed to init with %d", ret);
>           goto remove_msg_handler;
>       }
> diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
> index 5081d67e7f..e91566509f 100644
> --- a/hw/remote/proxy.c
> +++ b/hw/remote/proxy.c
> @@ -52,9 +52,20 @@ static void setup_irqfd(PCIProxyDev *dev)
>       PCIDevice *pci_dev = PCI_DEVICE(dev);
>       MPQemuMsg msg;
>       Error *local_err = NULL;
> +    int ret = 0;
>   
> -    event_notifier_init(&dev->intr, 0);
> -    event_notifier_init(&dev->resample, 0);
> +    ret = event_notifier_init(&dev->intr, 0);
> +    if (ret < 0) {
> +        error_report("Failed to init intr notifier: %s", strerror(-ret));
> +        return;
> +    }
> +
> +    ret = event_notifier_init(&dev->resample, 0);
> +    if (ret < 0) {
> +        error_report("Failed to init resample notifier: %s", strerror(-ret));
> +        event_notifier_cleanup(&dev->intr);
> +        return;
> +    }
>   
>       memset(&msg, 0, sizeof(MPQemuMsg));
>       msg.cmd = MPQEMU_CMD_SET_IRQFD;
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index e58a0169af..5c8f305653 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -180,7 +180,7 @@ static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
>           return false;
>       }
>   
> -    if (event_notifier_init(notifier, 0)) {
> +    if (event_notifier_init(notifier, 0) < 0) {
>           error_setg_errno(errp, errno,
>                            "vfio: Unable to init event notifier for irq (%d)",
>                            irq);
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 2251facb35..ce9c014e6a 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -418,7 +418,7 @@ static bool vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev,
>           return false;
>       }
>   
> -    if (event_notifier_init(notifier, 0)) {
> +    if (event_notifier_init(notifier, 0) < 0) {
>           error_setg_errno(errp, errno,
>                            "vfio: Unable to init event notifier for irq (%d)",
>                            irq);
> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> index 7b907b9360..66e02b15a4 100644
> --- a/hw/vfio/pci-quirks.c
> +++ b/hw/vfio/pci-quirks.c
> @@ -318,7 +318,7 @@ static VFIOIOEventFD *vfio_ioeventfd_init(VFIOPCIDevice *vdev,
>   
>       ioeventfd = g_malloc0(sizeof(*ioeventfd));
>   
> -    if (event_notifier_init(&ioeventfd->e, 0)) {
> +    if (event_notifier_init(&ioeventfd->e, 0) < 0) {
>           g_free(ioeventfd);
>           return NULL;
>       }
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 94c174a773..1945751ffd 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -70,7 +70,7 @@ static bool vfio_notifier_init(VFIOPCIDevice *vdev, EventNotifier *e,
>       }
>   
>       ret = event_notifier_init(e, 0);
> -    if (ret) {
> +    if (ret < 0) {
>           error_setg_errno(errp, -ret, "vfio_notifier_init %s failed", name);
>           return false;
>       }
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 2f8f11df86..9c7634e243 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -1075,13 +1075,13 @@ static int vhost_vdpa_svq_set_fds(struct vhost_dev *dev,
>       int r;
>   
>       r = event_notifier_init(&svq->hdev_kick, 0);
> -    if (r != 0) {
> +    if (r < 0) {
>           error_setg_errno(errp, -r, "Couldn't create kick event notifier");
>           goto err_init_hdev_kick;
>       }
>   
>       r = event_notifier_init(&svq->hdev_call, 0);
> -    if (r != 0) {
> +    if (r < 0) {
>           error_setg_errno(errp, -r, "Couldn't create call event notifier");
>           goto err_init_hdev_call;
>       }

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.

Re: [PATCH v2 2/4] hw/core/loader: fix error handling for get_image_size callers

[Clément Chigot]

On Wed, Mar 11, 2026 at 7:35 PM Trieu Huynh <vikingtc4@gmail.com> wrote:
>
> From: Trieu Huynh <vikingtc4@gmail.com>
>
> Check the return value of get_image_size() and report failures
> for non-mandatory files such as BIOS or FRU images.
>
> - Use ret < 0 to detect failures in getting image size.
> - No functional changes.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
> Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
>
> ---
> v2:
> - Keep error reporting as it is.
> - Remove exit on error since it is not mandatory.
> ---
>  hw/ipmi/ipmi_bmc_sim.c | 2 ++
>  hw/sparc/leon3.c       | 4 ++++
>  2 files changed, 6 insertions(+)
>
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 012e2ee4fe..fd875491f5 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -2561,6 +2561,8 @@ static void ipmi_fru_init(IPMIFru *fru)
>              g_free(fru->data);
>              fru->data = NULL;
>          }
> +    } else {
> +        error_report("Could not get file size '%s'", fru->filename);
>      }
>
>  out:
> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
> index 9147d56bd4..122366829d 100644
> --- a/hw/sparc/leon3.c
> +++ b/hw/sparc/leon3.c
> @@ -354,6 +354,10 @@ static void leon3_generic_hw_init(MachineState *machine)
>      } else {
>          bios_size = -1;
>      }
> +    if (bios_size < 0) {
> +        error_report("could not found or failed to get file size: '%s'",
> +                     filename);
> +    }

Honestly, I'm not sure what improvements that brings. This warning is
meaningless for Baremetal programs (launched without bios). Moreover,
an error handling is already there if "bios_size < 0" and -kernel is
not provided (just below).

A better approach could be to add support for "-bios none". With the
whole bios handling section (from "Allocate BIOS" up to
"g_free(filename);") could be wrapped under "if -bios != none".
But that differs slightly from your initial goal. So up to you.

>      if (bios_size > prom_size) {
>          error_report("could not load prom '%s': file too big", filename);
> --
> 2.43.0
>

Re: [PATCH v2 1/4] hw/core/loader: fix error handling for load_image_targphys callers

[Peter Maydell]

On Wed, 11 Mar 2026 at 18:35, Trieu Huynh <vikingtc4@gmail.com> wrote:
>
> From: Trieu Huynh <vikingtc4@gmail.com>
>
> Use QEMU's Error API to handle load_image_targphys() failures
> consistently across callers.
>
> - Use &error_fatal for callers that previously passed NULL, ensuring
> the process exits early on failure instead of continuing in an invalid
> state.
> - No functional changes.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
> Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
>
> ---
> v2:
> - Use &error_fatal instead of manual return value checks.
> - Remove redundant checks for ppc callers that already use
> &error_fatal.
> - Note: This replaces the "ret < 0" approach proposed in v1.

Hi; this patch breaks "make check" for me:

  13/1040 qtest+qtest-m68k - qemu:qtest-m68k/qom-test
           ERROR            1.35s   killed by signal 6 SIGABRT
 113/1040 qtest+qtest-m68k - qemu:qtest-m68k/test-hmp
           ERROR            1.36s   killed by signal 6 SIGABRT

This is because these tests check that every machine can
be started without errors (when run with the 'qtest' accelerator),
and now next-cube cannot:

$ ./build/clang/qemu-system-m68k -display none -machine next-cube -accel qtest
qemu-system-m68k: Could not open 'Rev_2.5_v66.bin': No such file or directory

Where the machine init code is calling load_image_targphys()
because the user passed in a particular filename to load,
then we definitely want to use error_fatal. But some of
these cases use these codepaths for "try to load the default
BIOS filename" and rely on ignoring the error if it doesn't
actually exist.

I think of the machines you change here, only next-cube
has this issue, and that change has other problems:

> diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
> index 26177c7b86..4bc8e72f3e 100644
> --- a/hw/m68k/next-cube.c
> +++ b/hw/m68k/next-cube.c
> @@ -1326,7 +1326,7 @@ static void next_cube_init(MachineState *machine)
>      memory_region_init_alias(&m->rom2, NULL, "next.rom2", &m->rom, 0x0,
>                               0x20000);
>      memory_region_add_subregion(sysmem, 0x0, &m->rom2);
> -    if (load_image_targphys(bios_name, 0x01000000, 0x20000, NULL) < 8) {
> +    if (load_image_targphys(bios_name, 0x01000000, 0x20000, &error_fatal) < 8) {

We were already doing an error check here, by looking at the return
value and insisting it was at least 8 bytes.

>          if (!qtest_enabled()) {
>              error_report("Failed to load firmware '%s'.", bios_name);

We could if we liked report the error to the user, by passing
an Error* to load_image_targphys(), reporting the error if it
is set, and otherwise reporting "file too short" (since that's
the other reason we'll get here).

The reason for the !qtest_enabled() here, by the way, is to
get the "report this error to the user if run in the normal way,
but don't report the error if this is one of the tests that
is doing the "check every machine can be started" test.)

>          }

thanks
-- PMM

Re: [PATCH v2 4/4] hw/pci/msix: fix error handling for msix_init callers

[Akihiko Odaki]

On 2026/03/12 3:34, Trieu Huynh wrote:
> From: Trieu Huynh <vikingtc4@gmail.com>
> 
> Check return value of msix_init() and return early on
> failure instead of continuing with invalid state.
> - Use ret < 0 to handle negative return value.
> - Use errp parameter to handle failure instead of NULL.
> - No functional changes.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
> Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
> 
> ---
> v2:
> - Use errp parameter to handle failure instead of NULL.
> ---
>   hw/net/igbvf.c         |  2 +-
>   hw/net/rocker/rocker.c |  2 +-
>   hw/pci/msix.c          |  2 +-
>   hw/scsi/megasas.c      | 18 +++++++++++++-----
>   hw/usb/hcd-xhci-pci.c  | 19 ++++++++++++++-----
>   5 files changed, 30 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/net/igbvf.c b/hw/net/igbvf.c
> index 48d56e43ac..9a165c7063 100644
> --- a/hw/net/igbvf.c
> +++ b/hw/net/igbvf.c
> @@ -260,7 +260,7 @@ static void igbvf_pci_realize(PCIDevice *dev, Error **errp)
>   
>       ret = msix_init(dev, IGBVF_MSIX_VEC_NUM, &s->msix, IGBVF_MSIX_BAR_IDX, 0,
>           &s->msix, IGBVF_MSIX_BAR_IDX, 0x2000, 0x70, errp);
> -    if (ret) {
> +    if (ret < 0) {
>           return;
>       }
>   
> diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
> index 4a7056bd45..910dce901b 100644
> --- a/hw/net/rocker/rocker.c
> +++ b/hw/net/rocker/rocker.c
> @@ -1228,7 +1228,7 @@ static int rocker_msix_init(Rocker *r, Error **errp)
>                       &r->msix_bar,
>                       ROCKER_PCI_MSIX_BAR_IDX, ROCKER_PCI_MSIX_PBA_OFFSET,
>                       0, errp);
> -    if (err) {
> +    if (err < 0) {
>           return err;
>       }
>   
> diff --git a/hw/pci/msix.c b/hw/pci/msix.c
> index b35476d057..1b23eaf100 100644
> --- a/hw/pci/msix.c
> +++ b/hw/pci/msix.c
> @@ -432,7 +432,7 @@ int msix_init_exclusive_bar(PCIDevice *dev, uint32_t nentries,
>                       0, &dev->msix_exclusive_bar,
>                       bar_nr, bar_pba_offset,
>                       0, errp);
> -    if (ret) {
> +    if (ret < 0) {
>           return ret;
>       }
>   
> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> index f62e420a91..b237ba7656 100644
> --- a/hw/scsi/megasas.c
> +++ b/hw/scsi/megasas.c
> @@ -2380,11 +2380,19 @@ static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
>       memory_region_init_io(&s->queue_io, OBJECT(s), &megasas_queue_ops, s,
>                             "megasas-queue", 0x40000);
>   
> -    if (megasas_use_msix(s) &&
> -        msix_init(dev, 15, &s->mmio_io, b->mmio_bar, 0x2000,
> -                  &s->mmio_io, b->mmio_bar, 0x3800, 0x68, NULL)) {
> -        /* TODO: check msix_init's error, and should fail on msix=on */
> -        s->msix = ON_OFF_AUTO_OFF;
> +    if (megasas_use_msix(s)) {
> +        Error *local_err = NULL;
> +        ret = msix_init(dev, 15, &s->mmio_io, b->mmio_bar, 0x2000,
> +                        &s->mmio_io, b->mmio_bar, 0x3800, 0x68, &local_err);
> +
> +        if (ret < 0) {
> +            if (s->msix == ON_OFF_AUTO_ON) {
> +                error_propagate(errp, local_err);

error_propagate() is discouraged in include/qapi/error.h because 
ERRP_GUARD() exists as an alternative so I thought of using it, but 
perhaps it may be a better to avoid having a local error variable 
altogether.

Instead, you can pass (s->msix == ON_OF_AUTO_ON ? errp : NULL) to 
msix_init() as the errp parameter.

Regards,
Akihiko Odaki

> +                return;
> +            }
> +            error_free(local_err);
> +            s->msix = ON_OFF_AUTO_OFF;
> +        }
>       }
>   
>       if (pci_is_express(dev)) {
> diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
> index aa570506fc..3bdde9b64a 100644
> --- a/hw/usb/hcd-xhci-pci.c
> +++ b/hw/usb/hcd-xhci-pci.c
> @@ -173,11 +173,20 @@ static void usb_xhci_pci_realize(struct PCIDevice *dev, Error **errp)
>       }
>   
>       if (s->msix != ON_OFF_AUTO_OFF) {
> -        /* TODO check for errors, and should fail when msix=on */
> -        msix_init(dev, s->xhci.numintrs,
> -                  &s->xhci.mem, 0, OFF_MSIX_TABLE,
> -                  &s->xhci.mem, 0, OFF_MSIX_PBA,
> -                  0x90, NULL);
> +        Error *local_err = NULL;
> +        ret = msix_init(dev, s->xhci.numintrs,
> +                        &s->xhci.mem, 0, OFF_MSIX_TABLE,
> +                        &s->xhci.mem, 0, OFF_MSIX_PBA,
> +                        0x90, &local_err);
> +
> +        if (ret < 0) {
> +            if (s->msix == ON_OFF_AUTO_ON) {
> +                error_propagate(errp, local_err);
> +                return;
> +            }
> +            error_free(local_err);
> +            s->msix = ON_OFF_AUTO_OFF;
> +        }
>       }
>       s->xhci.as = pci_get_address_space(dev);
>   }

Re: [PATCH v2 3/4] util/event_notifier: fix error handling for event_notifier_init callers

[Maciej S. Szmigiero]

On 11.03.2026 19:34, Trieu Huynh wrote:
> From: Trieu Huynh <vikingtc4@gmail.com>
> 
> Check return value of event_notifier_init() and return early on
> failure instead of continuing with invalid state.
> - Use ret < 0 to handle negative return value.
> - No functional changes.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
> Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
> Acked-by: Anthony Krowiak <akrowiak@linux.ibm.com>
> Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
> ---
>   hw/hyperv/hyperv.c     |  4 ++--
>   hw/hyperv/vmbus.c      |  4 ++--
>   hw/remote/proxy.c      | 15 +++++++++++++--
>   hw/vfio/ap.c           |  2 +-
>   hw/vfio/ccw.c          |  2 +-
>   hw/vfio/pci-quirks.c   |  2 +-
>   hw/vfio/pci.c          |  2 +-
>   hw/virtio/vhost-vdpa.c |  4 ++--
>   8 files changed, 23 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
> index 27e323a819..aa278b179e 100644
> --- a/hw/hyperv/hyperv.c
> +++ b/hw/hyperv/hyperv.c
> @@ -439,7 +439,7 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
>           sint_route->staged_msg->cb_data = cb_data;
>   
>           r = event_notifier_init(ack_notifier, false);
> -        if (r) {
> +        if (r < 0) {
>               goto cleanup_err_sint;
>           }
>           event_notifier_set_handler(ack_notifier, sint_ack_handler);
> @@ -453,7 +453,7 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
>   
>       /* We need to setup a GSI for this SintRoute */
>       r = event_notifier_init(&sint_route->sint_set_notifier, false);
> -    if (r) {
> +    if (r < 0) {
>           goto cleanup_err_sint;
>       }
>   
> diff --git a/hw/hyperv/vmbus.c b/hw/hyperv/vmbus.c
> index 64abe4c4c1..5388f4277f 100644
> --- a/hw/hyperv/vmbus.c
> +++ b/hw/hyperv/vmbus.c
> @@ -1432,7 +1432,7 @@ static void open_channel(VMBusChannel *chan)
>           goto put_gpadl;
>       }
>   
> -    if (event_notifier_init(&chan->notifier, 0)) {
> +    if (event_notifier_init(&chan->notifier, 0) < 0) {
>           goto put_gpadl;
>       }
>   
> @@ -2450,7 +2450,7 @@ static void vmbus_realize(BusState *bus, Error **errp)
>       }
>   
>       ret = event_notifier_init(&vmbus->notifier, 0);
> -    if (ret != 0) {
> +    if (ret < 0) {
>           error_setg(errp, "event notifier failed to init with %d", ret);
>           goto remove_msg_handler;
>       }

Reviewed-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> # for the Hyper-V part

Thanks,
Maciej

Re: [PATCH v2 3/4] util/event_notifier: fix error handling for event_notifier_init callers

[Matthew Rosato]

On 3/11/26 2:34 PM, Trieu Huynh wrote:
> From: Trieu Huynh <vikingtc4@gmail.com>
> 
> Check return value of event_notifier_init() and return early on
> failure instead of continuing with invalid state.
> - Use ret < 0 to handle negative return value.
> - No functional changes.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
> Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
> Acked-by: Anthony Krowiak <akrowiak@linux.ibm.com>
> Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
> ---
>  hw/hyperv/hyperv.c     |  4 ++--
>  hw/hyperv/vmbus.c      |  4 ++--
>  hw/remote/proxy.c      | 15 +++++++++++++--
>  hw/vfio/ap.c           |  2 +-
>  hw/vfio/ccw.c          |  2 +-
>  hw/vfio/pci-quirks.c   |  2 +-
>  hw/vfio/pci.c          |  2 +-
>  hw/virtio/vhost-vdpa.c |  4 ++--
>  8 files changed, 23 insertions(+), 12 deletions(-)
> 

[...]

>      memset(&msg, 0, sizeof(MPQemuMsg));
>      msg.cmd = MPQEMU_CMD_SET_IRQFD;
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index e58a0169af..5c8f305653 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -180,7 +180,7 @@ static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
>          return false;
>      }
>  
> -    if (event_notifier_init(notifier, 0)) {
> +    if (event_notifier_init(notifier, 0) < 0) {
>          error_setg_errno(errp, errno,
>                           "vfio: Unable to init event notifier for irq (%d)",
>                           irq);
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 2251facb35..ce9c014e6a 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -418,7 +418,7 @@ static bool vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev,
>          return false;
>      }
>  
> -    if (event_notifier_init(notifier, 0)) {
> +    if (event_notifier_init(notifier, 0) < 0) {
>          error_setg_errno(errp, errno,
>                           "vfio: Unable to init event notifier for irq (%d)",
>                           irq);

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

[PATCH v3 v3 0/4] Fix error handling for callers of load_image_targphys, get_image_size, event_notifier_init, msix_init

[Trieu Huynh]

Changes in v3:
- Patch 1: For next-cube, use a local Error* instead of &error_fatal
so that load failures are reported only when !qtest_enabled(),
matching the existing guard. Distinguishes "file not found" (error
set) from "file too short" (no error, size < 8) in next-cube.
(pointed out by Peter Maydell)
- Patch 2: Drop leon3.c change: warning is meaningless for baremetal
programs where no BIOS is expected, and error handling already
exists downstream. (pointed out by Clément Chigot)
- Patch 3: No code changes. Add Reviewed-by tags.
- Patch 4: Replace error_propagate() with conditional errp passing:
(s->msix == ON_OFF_AUTO_ON ? errp : NULL), avoiding the need for
a local error variable. (pointed out by Akihiko Odaki)

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413

Trieu Huynh (4):
  hw/core/loader: fix error handling for load_image_targphys callers
  hw/core/loader: fix error handling for get_image_size callers
  util/event_notifier: fix error handling for event_notifier_init
    callers
  hw/pci/msix: fix error handling for msix_init callers

 hw/alpha/dp264.c       |  2 +-
 hw/hppa/machine.c      |  2 +-
 hw/hyperv/hyperv.c     |  4 ++--
 hw/hyperv/vmbus.c      |  4 ++--
 hw/ipmi/ipmi_bmc_sim.c |  2 ++
 hw/m68k/next-cube.c    | 11 +++++++++--
 hw/m68k/q800.c         |  2 +-
 hw/m68k/virt.c         |  2 +-
 hw/microblaze/boot.c   |  3 ++-
 hw/net/igbvf.c         |  2 +-
 hw/net/rocker/rocker.c |  2 +-
 hw/pci/msix.c          |  2 +-
 hw/remote/proxy.c      | 15 +++++++++++++--
 hw/scsi/megasas.c      | 16 +++++++++++-----
 hw/usb/hcd-xhci-pci.c  | 16 +++++++++++-----
 hw/vfio/ap.c           |  2 +-
 hw/vfio/ccw.c          |  2 +-
 hw/vfio/pci-quirks.c   |  2 +-
 hw/vfio/pci.c          |  2 +-
 hw/virtio/vhost-vdpa.c |  4 ++--
 20 files changed, 65 insertions(+), 32 deletions(-)

-- 
2.43.0

[PATCH v3 v3 1/4] hw/core/loader: fix error handling for load_image_targphys callers

[Trieu Huynh]

Use QEMU's Error API to handle load_image_targphys() failures
consistently across callers.

- Use &error_fatal for callers that previously passed NULL, ensuring
the process exits early on failure instead of continuing in an invalid
state.
- No functional changes.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
---
 hw/alpha/dp264.c     |  2 +-
 hw/hppa/machine.c    |  2 +-
 hw/m68k/next-cube.c  | 11 +++++++++--
 hw/m68k/q800.c       |  2 +-
 hw/m68k/virt.c       |  2 +-
 hw/microblaze/boot.c |  3 ++-
 6 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 98219f0456..2ab3c14747 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -190,7 +190,7 @@ static void clipper_init(MachineState *machine)
             /* Put the initrd image as high in memory as possible.  */
             initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
             load_image_targphys(initrd_filename, initrd_base,
-                                ram_size - initrd_base, NULL);
+                                ram_size - initrd_base, &error_fatal);
 
             address_space_stq_le(&address_space_memory, param_offset + 0x100,
                                  initrd_base + 0xfffffc0000000000ULL,
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index ec63dc1297..99a4c22c73 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -507,7 +507,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
             }
 
             load_image_targphys(initrd_filename, initrd_base, initrd_size,
-                                NULL);
+                                &error_fatal);
             cpu[0]->env.initrd_base = initrd_base;
             cpu[0]->env.initrd_end  = initrd_base + initrd_size;
         }
diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
index 26177c7b86..4bfe5bcf56 100644
--- a/hw/m68k/next-cube.c
+++ b/hw/m68k/next-cube.c
@@ -1326,9 +1326,16 @@ static void next_cube_init(MachineState *machine)
     memory_region_init_alias(&m->rom2, NULL, "next.rom2", &m->rom, 0x0,
                              0x20000);
     memory_region_add_subregion(sysmem, 0x0, &m->rom2);
-    if (load_image_targphys(bios_name, 0x01000000, 0x20000, NULL) < 8) {
+    Error *local_err = NULL;
+    if (load_image_targphys(bios_name, 0x01000000, 0x20000, &local_err) < 8) {
         if (!qtest_enabled()) {
-            error_report("Failed to load firmware '%s'.", bios_name);
+            if (local_err) {
+                error_report_err(local_err);
+            } else {
+                error_report("Firmware image '%s' is too short.", bios_name);
+            }
+        } else {
+            error_free(local_err);
         }
     } else {
         uint8_t *ptr;
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index ded531394e..c0d78eb7d7 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -633,7 +633,7 @@ static void q800_machine_init(MachineState *machine)
 
             initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
             load_image_targphys(initrd_filename, initrd_base,
-                                ram_size - initrd_base, NULL);
+                                ram_size - initrd_base, &error_fatal);
             BOOTINFO2(param_ptr, BI_RAMDISK, initrd_base,
                       initrd_size);
         } else {
diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c
index e67900c727..ffe6e23415 100644
--- a/hw/m68k/virt.c
+++ b/hw/m68k/virt.c
@@ -292,7 +292,7 @@ static void virt_init(MachineState *machine)
 
             initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
             load_image_targphys(initrd_filename, initrd_base,
-                                ram_size - initrd_base, NULL);
+                                ram_size - initrd_base, &error_fatal);
             BOOTINFO2(param_ptr, BI_RAMDISK, initrd_base,
                       initrd_size);
         } else {
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index a6f9ebab90..4ad5ffd34b 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -38,6 +38,7 @@
 #include "hw/core/loader.h"
 #include "elf.h"
 #include "qemu/cutils.h"
+#include "qapi/error.h"
 
 #include "boot.h"
 
@@ -171,7 +172,7 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu, bool is_little_endian,
         /* Not an ELF image nor an u-boot image, try a RAW image.  */
         if (kernel_size < 0) {
             kernel_size = load_image_targphys(kernel_filename, ddr_base,
-                                              ramsize, NULL);
+                                              ramsize, &error_fatal);
             boot_info.bootstrap_pc = ddr_base;
             high = (ddr_base + kernel_size + 3) & ~3;
         }
-- 
2.43.0

[PATCH v3 v3 2/4] hw/core/loader: fix error handling for get_image_size callers

[Trieu Huynh]

Check the return value of get_image_size() and report failures
for non-mandatory file such as FRU image.

- Use ret < 0 to detect failures in getting image size.
- No functional changes.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
---
 hw/ipmi/ipmi_bmc_sim.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 012e2ee4fe..fd875491f5 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -2561,6 +2561,8 @@ static void ipmi_fru_init(IPMIFru *fru)
             g_free(fru->data);
             fru->data = NULL;
         }
+    } else {
+        error_report("Could not get file size '%s'", fru->filename);
     }
 
 out:
-- 
2.43.0

[PATCH v3 v3 3/4] util/event_notifier: fix error handling for event_notifier_init callers

[Trieu Huynh]

Check return value of event_notifier_init() and return early on
failure instead of continuing with invalid state.
- Use ret < 0 to handle negative return value.
- No functional changes.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
Acked-by: Anthony Krowiak <akrowiak@linux.ibm.com>
Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
Reviewed-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> # for the Hyper-V part
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
---
 hw/hyperv/hyperv.c     |  4 ++--
 hw/hyperv/vmbus.c      |  4 ++--
 hw/remote/proxy.c      | 15 +++++++++++++--
 hw/vfio/ap.c           |  2 +-
 hw/vfio/ccw.c          |  2 +-
 hw/vfio/pci-quirks.c   |  2 +-
 hw/vfio/pci.c          |  2 +-
 hw/virtio/vhost-vdpa.c |  4 ++--
 8 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
index 27e323a819..aa278b179e 100644
--- a/hw/hyperv/hyperv.c
+++ b/hw/hyperv/hyperv.c
@@ -439,7 +439,7 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
         sint_route->staged_msg->cb_data = cb_data;
 
         r = event_notifier_init(ack_notifier, false);
-        if (r) {
+        if (r < 0) {
             goto cleanup_err_sint;
         }
         event_notifier_set_handler(ack_notifier, sint_ack_handler);
@@ -453,7 +453,7 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
 
     /* We need to setup a GSI for this SintRoute */
     r = event_notifier_init(&sint_route->sint_set_notifier, false);
-    if (r) {
+    if (r < 0) {
         goto cleanup_err_sint;
     }
 
diff --git a/hw/hyperv/vmbus.c b/hw/hyperv/vmbus.c
index 64abe4c4c1..5388f4277f 100644
--- a/hw/hyperv/vmbus.c
+++ b/hw/hyperv/vmbus.c
@@ -1432,7 +1432,7 @@ static void open_channel(VMBusChannel *chan)
         goto put_gpadl;
     }
 
-    if (event_notifier_init(&chan->notifier, 0)) {
+    if (event_notifier_init(&chan->notifier, 0) < 0) {
         goto put_gpadl;
     }
 
@@ -2450,7 +2450,7 @@ static void vmbus_realize(BusState *bus, Error **errp)
     }
 
     ret = event_notifier_init(&vmbus->notifier, 0);
-    if (ret != 0) {
+    if (ret < 0) {
         error_setg(errp, "event notifier failed to init with %d", ret);
         goto remove_msg_handler;
     }
diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
index 5081d67e7f..e91566509f 100644
--- a/hw/remote/proxy.c
+++ b/hw/remote/proxy.c
@@ -52,9 +52,20 @@ static void setup_irqfd(PCIProxyDev *dev)
     PCIDevice *pci_dev = PCI_DEVICE(dev);
     MPQemuMsg msg;
     Error *local_err = NULL;
+    int ret = 0;
 
-    event_notifier_init(&dev->intr, 0);
-    event_notifier_init(&dev->resample, 0);
+    ret = event_notifier_init(&dev->intr, 0);
+    if (ret < 0) {
+        error_report("Failed to init intr notifier: %s", strerror(-ret));
+        return;
+    }
+
+    ret = event_notifier_init(&dev->resample, 0);
+    if (ret < 0) {
+        error_report("Failed to init resample notifier: %s", strerror(-ret));
+        event_notifier_cleanup(&dev->intr);
+        return;
+    }
 
     memset(&msg, 0, sizeof(MPQemuMsg));
     msg.cmd = MPQEMU_CMD_SET_IRQFD;
diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index e58a0169af..5c8f305653 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -180,7 +180,7 @@ static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
         return false;
     }
 
-    if (event_notifier_init(notifier, 0)) {
+    if (event_notifier_init(notifier, 0) < 0) {
         error_setg_errno(errp, errno,
                          "vfio: Unable to init event notifier for irq (%d)",
                          irq);
diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 2251facb35..ce9c014e6a 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -418,7 +418,7 @@ static bool vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev,
         return false;
     }
 
-    if (event_notifier_init(notifier, 0)) {
+    if (event_notifier_init(notifier, 0) < 0) {
         error_setg_errno(errp, errno,
                          "vfio: Unable to init event notifier for irq (%d)",
                          irq);
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 7b907b9360..66e02b15a4 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -318,7 +318,7 @@ static VFIOIOEventFD *vfio_ioeventfd_init(VFIOPCIDevice *vdev,
 
     ioeventfd = g_malloc0(sizeof(*ioeventfd));
 
-    if (event_notifier_init(&ioeventfd->e, 0)) {
+    if (event_notifier_init(&ioeventfd->e, 0) < 0) {
         g_free(ioeventfd);
         return NULL;
     }
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 94c174a773..1945751ffd 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -70,7 +70,7 @@ static bool vfio_notifier_init(VFIOPCIDevice *vdev, EventNotifier *e,
     }
 
     ret = event_notifier_init(e, 0);
-    if (ret) {
+    if (ret < 0) {
         error_setg_errno(errp, -ret, "vfio_notifier_init %s failed", name);
         return false;
     }
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 2f8f11df86..9c7634e243 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1075,13 +1075,13 @@ static int vhost_vdpa_svq_set_fds(struct vhost_dev *dev,
     int r;
 
     r = event_notifier_init(&svq->hdev_kick, 0);
-    if (r != 0) {
+    if (r < 0) {
         error_setg_errno(errp, -r, "Couldn't create kick event notifier");
         goto err_init_hdev_kick;
     }
 
     r = event_notifier_init(&svq->hdev_call, 0);
-    if (r != 0) {
+    if (r < 0) {
         error_setg_errno(errp, -r, "Couldn't create call event notifier");
         goto err_init_hdev_call;
     }
-- 
2.43.0

[PATCH v3 v3 4/4] hw/pci/msix: fix error handling for msix_init callers

[Trieu Huynh]

Check return value of msix_init() and return early on
failure instead of continuing with invalid state.
- Use ret < 0 to handle negative return value.
- Use errp parameter to handle failure instead of NULL.
- No functional changes.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
---
 hw/net/igbvf.c         |  2 +-
 hw/net/rocker/rocker.c |  2 +-
 hw/pci/msix.c          |  2 +-
 hw/scsi/megasas.c      | 16 +++++++++++-----
 hw/usb/hcd-xhci-pci.c  | 16 +++++++++++-----
 5 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/hw/net/igbvf.c b/hw/net/igbvf.c
index 48d56e43ac..9a165c7063 100644
--- a/hw/net/igbvf.c
+++ b/hw/net/igbvf.c
@@ -260,7 +260,7 @@ static void igbvf_pci_realize(PCIDevice *dev, Error **errp)
 
     ret = msix_init(dev, IGBVF_MSIX_VEC_NUM, &s->msix, IGBVF_MSIX_BAR_IDX, 0,
         &s->msix, IGBVF_MSIX_BAR_IDX, 0x2000, 0x70, errp);
-    if (ret) {
+    if (ret < 0) {
         return;
     }
 
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index 4a7056bd45..910dce901b 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -1228,7 +1228,7 @@ static int rocker_msix_init(Rocker *r, Error **errp)
                     &r->msix_bar,
                     ROCKER_PCI_MSIX_BAR_IDX, ROCKER_PCI_MSIX_PBA_OFFSET,
                     0, errp);
-    if (err) {
+    if (err < 0) {
         return err;
     }
 
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index b35476d057..1b23eaf100 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -432,7 +432,7 @@ int msix_init_exclusive_bar(PCIDevice *dev, uint32_t nentries,
                     0, &dev->msix_exclusive_bar,
                     bar_nr, bar_pba_offset,
                     0, errp);
-    if (ret) {
+    if (ret < 0) {
         return ret;
     }
 
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index f62e420a91..a29742d449 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2380,11 +2380,17 @@ static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
     memory_region_init_io(&s->queue_io, OBJECT(s), &megasas_queue_ops, s,
                           "megasas-queue", 0x40000);
 
-    if (megasas_use_msix(s) &&
-        msix_init(dev, 15, &s->mmio_io, b->mmio_bar, 0x2000,
-                  &s->mmio_io, b->mmio_bar, 0x3800, 0x68, NULL)) {
-        /* TODO: check msix_init's error, and should fail on msix=on */
-        s->msix = ON_OFF_AUTO_OFF;
+    if (megasas_use_msix(s)) {
+        ret = msix_init(dev, 15, &s->mmio_io, b->mmio_bar, 0x2000,
+                        &s->mmio_io, b->mmio_bar, 0x3800, 0x68,
+                        s->msix == ON_OFF_AUTO_ON ? errp : NULL);
+
+        if (ret < 0) {
+            if (s->msix == ON_OFF_AUTO_ON) {
+                return;
+            }
+            s->msix = ON_OFF_AUTO_OFF;
+        }
     }
 
     if (pci_is_express(dev)) {
diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
index aa570506fc..c5446a4a5e 100644
--- a/hw/usb/hcd-xhci-pci.c
+++ b/hw/usb/hcd-xhci-pci.c
@@ -173,11 +173,17 @@ static void usb_xhci_pci_realize(struct PCIDevice *dev, Error **errp)
     }
 
     if (s->msix != ON_OFF_AUTO_OFF) {
-        /* TODO check for errors, and should fail when msix=on */
-        msix_init(dev, s->xhci.numintrs,
-                  &s->xhci.mem, 0, OFF_MSIX_TABLE,
-                  &s->xhci.mem, 0, OFF_MSIX_PBA,
-                  0x90, NULL);
+        ret = msix_init(dev, s->xhci.numintrs,
+                        &s->xhci.mem, 0, OFF_MSIX_TABLE,
+                        &s->xhci.mem, 0, OFF_MSIX_PBA,
+                        0x90, s->msix == ON_OFF_AUTO_ON ? errp : NULL);
+
+        if (ret < 0) {
+            if (s->msix == ON_OFF_AUTO_ON) {
+                return;
+            }
+            s->msix = ON_OFF_AUTO_OFF;
+        }
     }
     s->xhci.as = pci_get_address_space(dev);
 }
-- 
2.43.0