[libvirt PATCH] qemu: Improve error message for failed firmware autoselection

Andrea Bolognani posted 1 patch 7 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20230922132636.361169-1-abologna@redhat.com
src/qemu/qemu_firmware.c                                        | 2 +-
.../firmware-auto-efi-loader-path-nonstandard.x86_64-latest.err | 2 +-
...rmware-auto-efi-nvram-template-nonstandard.x86_64-latest.err | 2 +-
.../firmware-auto-efi-rw-abi-update.x86_64-latest.err           | 2 +-
tests/qemuxml2argvdata/firmware-auto-efi-rw.x86_64-latest.err   | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
[libvirt PATCH] qemu: Improve error message for failed firmware autoselection
Posted by Andrea Bolognani 7 months, 1 week ago
The current message can be misleading, because it seems to suggest
that no firmware of the requested type is available on the system.

What actually happens most of the time, however, is that despite
having multiple firmwares of the right type to choose from, none
of them is suitable because of lacking some specific feature or
being incompatible with some setting that the user has explicitly
enabled.

Providing an error message that describes exactly the problem is
not feasible, since we would have to list each candidate along
with the reason why we rejected it, which would get out of hand
quickly.

As a small but hopefully helpful improvement over the current
situation, reword the error message to make it clearer that the
culprit is not necessarily the firmware type, but rather the
overall domain configuration.

Suggested-by: Michael Kjörling <7d1340278307@ewoof.net>
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/qemu/qemu_firmware.c                                        | 2 +-
 .../firmware-auto-efi-loader-path-nonstandard.x86_64-latest.err | 2 +-
 ...rmware-auto-efi-nvram-template-nonstandard.x86_64-latest.err | 2 +-
 .../firmware-auto-efi-rw-abi-update.x86_64-latest.err           | 2 +-
 tests/qemuxml2argvdata/firmware-auto-efi-rw.x86_64-latest.err   | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 3dcd139a47..d39e61d071 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1854,7 +1854,7 @@ qemuFirmwareFillDomain(virQEMUDriver *driver,
             }
         } else {
             virReportError(VIR_ERR_OPERATION_FAILED,
-                           _("Unable to find any firmware to satisfy '%1$s'"),
+                           _("Unable to find '%1$s' firmware that is compatible with the current configuration"),
                            virDomainOsDefFirmwareTypeToString(def->os.firmware));
             return -1;
         }
diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-loader-path-nonstandard.x86_64-latest.err b/tests/qemuxml2argvdata/firmware-auto-efi-loader-path-nonstandard.x86_64-latest.err
index 4cfde1bd2e..3edb2b3451 100644
--- a/tests/qemuxml2argvdata/firmware-auto-efi-loader-path-nonstandard.x86_64-latest.err
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-loader-path-nonstandard.x86_64-latest.err
@@ -1 +1 @@
-operation failed: Unable to find any firmware to satisfy 'efi'
+operation failed: Unable to find 'efi' firmware that is compatible with the current configuration
diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-nvram-template-nonstandard.x86_64-latest.err b/tests/qemuxml2argvdata/firmware-auto-efi-nvram-template-nonstandard.x86_64-latest.err
index 4cfde1bd2e..3edb2b3451 100644
--- a/tests/qemuxml2argvdata/firmware-auto-efi-nvram-template-nonstandard.x86_64-latest.err
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-nvram-template-nonstandard.x86_64-latest.err
@@ -1 +1 @@
-operation failed: Unable to find any firmware to satisfy 'efi'
+operation failed: Unable to find 'efi' firmware that is compatible with the current configuration
diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-rw-abi-update.x86_64-latest.err b/tests/qemuxml2argvdata/firmware-auto-efi-rw-abi-update.x86_64-latest.err
index 4cfde1bd2e..3edb2b3451 100644
--- a/tests/qemuxml2argvdata/firmware-auto-efi-rw-abi-update.x86_64-latest.err
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-rw-abi-update.x86_64-latest.err
@@ -1 +1 @@
-operation failed: Unable to find any firmware to satisfy 'efi'
+operation failed: Unable to find 'efi' firmware that is compatible with the current configuration
diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-rw.x86_64-latest.err b/tests/qemuxml2argvdata/firmware-auto-efi-rw.x86_64-latest.err
index 4cfde1bd2e..3edb2b3451 100644
--- a/tests/qemuxml2argvdata/firmware-auto-efi-rw.x86_64-latest.err
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-rw.x86_64-latest.err
@@ -1 +1 @@
-operation failed: Unable to find any firmware to satisfy 'efi'
+operation failed: Unable to find 'efi' firmware that is compatible with the current configuration
-- 
2.41.0

Re: [libvirt PATCH] qemu: Improve error message for failed firmware autoselection
Posted by Michal Prívozník 7 months, 1 week ago
On 9/22/23 15:26, Andrea Bolognani wrote:
> The current message can be misleading, because it seems to suggest
> that no firmware of the requested type is available on the system.
> 
> What actually happens most of the time, however, is that despite
> having multiple firmwares of the right type to choose from, none
> of them is suitable because of lacking some specific feature or
> being incompatible with some setting that the user has explicitly
> enabled.
> 
> Providing an error message that describes exactly the problem is
> not feasible, since we would have to list each candidate along
> with the reason why we rejected it, which would get out of hand
> quickly.
> 
> As a small but hopefully helpful improvement over the current
> situation, reword the error message to make it clearer that the
> culprit is not necessarily the firmware type, but rather the
> overall domain configuration.
> 
> Suggested-by: Michael Kjörling <7d1340278307@ewoof.net>
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  src/qemu/qemu_firmware.c                                        | 2 +-
>  .../firmware-auto-efi-loader-path-nonstandard.x86_64-latest.err | 2 +-
>  ...rmware-auto-efi-nvram-template-nonstandard.x86_64-latest.err | 2 +-
>  .../firmware-auto-efi-rw-abi-update.x86_64-latest.err           | 2 +-
>  tests/qemuxml2argvdata/firmware-auto-efi-rw.x86_64-latest.err   | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

And maybe we can document what us, libvirt developers, would do to debug
this? I mean, we can put somewhere in a kbase article that we would
enable debug logs and then check log messages where individual reasons
for rejecting each fw are logged.

Michal

Re: [libvirt PATCH] qemu: Improve error message for failed firmware autoselection
Posted by Andrea Bolognani 7 months, 1 week ago
On Fri, Sep 22, 2023 at 03:44:55PM +0200, Michal Prívozník wrote:
> On 9/22/23 15:26, Andrea Bolognani wrote:
> >  src/qemu/qemu_firmware.c                                        | 2 +-
> >  .../firmware-auto-efi-loader-path-nonstandard.x86_64-latest.err | 2 +-
> >  ...rmware-auto-efi-nvram-template-nonstandard.x86_64-latest.err | 2 +-
> >  .../firmware-auto-efi-rw-abi-update.x86_64-latest.err           | 2 +-
> >  tests/qemuxml2argvdata/firmware-auto-efi-rw.x86_64-latest.err   | 2 +-
> >  5 files changed, 5 insertions(+), 5 deletions(-)
>
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Pushed, thanks.

> And maybe we can document what us, libvirt developers, would do to debug
> this? I mean, we can put somewhere in a kbase article that we would
> enable debug logs and then check log messages where individual reasons
> for rejecting each fw are logged.

Can you think of an existing page where it would fit?

Honestly it would probably make sense to have an entire kbase
dedicated to firmware selection, with recommendations, best
practices, gotchas and so on. Maybe we could then include the
debugging steps as part of it?

Note that the current output, even with debugging enabled, is not
very easy to follow, so before suggesting that people go look at it I
would probably want to improve upon it.

Damn, I think you might just have nerd-sniped me into working on
this! Let me add it to my to-do list :D

-- 
Andrea Bolognani / Red Hat / Virtualization