On 11/03/26 00:32, Aditya Gupta wrote:

On 10/03/26 23:57, Trieu Huynh wrote:
From: Trieu Huynh <[email protected]>

Check return value of load_image_targphys() 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 <[email protected]>

<...snip...>
  diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 1513575b8f..aecffb211b 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1068,14 +1068,21 @@ static void pnv_init(MachineState *machine)
          exit(1);
      }
  -    load_image_targphys(fw_filename, pnv->fw_load_addr, FW_MAX_SIZE,
-                        &error_fatal);
+    if (load_image_targphys(fw_filename, pnv->fw_load_addr, FW_MAX_SIZE,
+                        &error_fatal) < 0) {
+        error_report("Could not load OPAL firmware '%s'", fw_filename);
+        exit(1);
+    }
      g_free(fw_filename);
        /* load kernel */
      if (machine->kernel_filename) {
-        load_image_targphys(machine->kernel_filename,
-                            KERNEL_LOAD_ADDR, KERNEL_MAX_SIZE, &error_fatal);
+        if (load_image_targphys(machine->kernel_filename,
+                        KERNEL_LOAD_ADDR, KERNEL_MAX_SIZE, &error_fatal) < 0) {
+            error_report("Could not load kernel '%s'",
+                         machine->kernel_filename);
+            exit(1);
+        }
      }
        /* load initrd */
@@ -1084,6 +1091,11 @@ static void pnv_init(MachineState *machine)
          pnv->initrd_size = load_image_targphys(machine->initrd_filename,
pnv->initrd_base,
INITRD_MAX_SIZE, &error_fatal);
+        if (pnv->initrd_size < 0) {
+            error_report("Could not load initrd '%s'",
+                         machine->initrd_filename);
+            exit(1);
+        }
      }
        /* load dtb if passed */


Thanks for the patch, but as balaton said, atleast in this case, the condition is not needed since it's have error_fatal.

Previously the if checks you added used to there, these were recently changed to not have the conditions,
by this patch from vishal:

    commit cd274e83d50ba52ede62d2a8ea0f0ae7cb1ef469
    Author: Vishal Chourasia <[email protected]>
    Date:   Fri Oct 24 18:36:03 2025 +0530

    hw/ppc: Pass error_fatal to load_image_targphys()

    Pass error_fatal to load_image_targphys() calls in ppc machine initialization
    to capture detailed error information when loading firmware, kernel,
    and initrd images.


To add to this, it might be better to pass an error object to load_image_targphys, or error_fatal if we are exiting in the if condition. That gives more details for why the load failed.

The NULL arguments were added in this: https://lore.kernel.org/qemu-devel/[email protected]/

That was because it was intended to not affect other machines, and hence it stayed with default
behavior of ignoring return value of load_image_targphys

This can be changed to use similar to PPC's approach to use error_fatal, if that's intended.



Thanks,
- Aditya G


Reply via email to