Re: [PATCH 1/2] target/riscv/kvm: improve 'init_multiext_cfg' error msg

[Philippe Mathieu-Daudé]

Hi Daniel,

On 3/10/23 13:32, Daniel Henrique Barboza wrote:
> Our error message is returning the value of 'ret', which will be always
> -1 in case of error, and will not be that useful:
>
> qemu-system-riscv64: Unable to read ISA_EXT KVM register ssaia, error -1
>
> Improve the error message by outputting 'errno' instead of 'ret'. Use
> strerrorname_np() to output the error name instead of the error code.
> This will give us what we need to know right away:
>
> qemu-system-riscv64: Unable to read ISA_EXT KVM register ssaia, error code: 
> ENOENT
>
> Use "error_setg(&error_fatal, ..." since it'll both print the error and
> do an exit(EXIT_FAILURE) in one single call, allowing us to remove
> error_report() and exit().
>
> Finally, given that we're going to exit(1) in this condition instead of
> attempting to recover, remove the 'kvm_riscv_destroy_scratch_vcpu()'
> call.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>   target/riscv/kvm/kvm-cpu.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index c6615cb807..847cb2876a 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -791,10 +791,9 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, 
> KVMScratchCPU *kvmcpu)
>                   multi_ext_cfg->supported = false;
>                   val = false;
>               } else {
> -                error_report("Unable to read ISA_EXT KVM register %s, "
> -                             "error %d", multi_ext_cfg->name, ret);
> -                kvm_riscv_destroy_scratch_vcpu(kvmcpu);
> -                exit(EXIT_FAILURE);
> +                error_setg(&error_fatal, "Unable to read ISA_EXT "

See the documentation added in include/qapi/error.h by commit
10303f04b9 ("error: Improve documentation some more"):

  * Please don't error_setg(&error_fatal, ...), use error_report() and
  * exit(), because that's more obvious.

> +                           "KVM register %s, error code: %s",
> +                           multi_ext_cfg->name, strerrorname_np(errno));
>               }
>           } else {
>               multi_ext_cfg->supported = true;