Re: [PATCH] target/riscv/kvm: do not use non-portable strerrorname_np()

[Daniel Henrique Barboza <dbarboza@ventanamicro.com>]

On 12/18/23 13:22, Natanael Copa wrote:
> strerrorname_np is non-portable and breaks building with musl libc.
> 
> Use strerror(errno) instead, like we do other places.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: commit 082e9e4a58ba (target/riscv/kvm: improve 'init_multiext_cfg' error msg)
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2041
> Buglink: https://gitlab.alpinelinux.org/alpine/aports/-/issues/15541
> Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>
> ---
>   target/riscv/kvm/kvm-cpu.c | 18 ++++++++----------
>   1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 45b6cf1cfa..117e33cf90 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -832,9 +832,8 @@ static void kvm_riscv_read_multiext_legacy(RISCVCPU *cpu,
>                   multi_ext_cfg->supported = false;
>                   val = false;
>               } else {
> -                error_report("Unable to read ISA_EXT KVM register %s, "
> -                             "error code: %s", multi_ext_cfg->name,
> -                             strerrorname_np(errno));
> +                error_report("Unable to read ISA_EXT KVM register %s: %s",
> +                             multi_ext_cfg->name, strerror(errno));


The reason I did this change, as described in 082e9e4a58ba mentioned in the commit
message, was precisely to avoid things like this:

qemu-system-riscv64: Unable to read ISA_EXT KVM register ssaia, error: no such file or directory
  
The generic description of the error works well with file descriptors and so on but it's
weird in the KVM context. This patch is re-introducing it.

If strerrorname_np() is non-portable I believe we're better off dealing with the numeric
errno than with its generic description. I.e:


> +                error_report("Unable to read ISA_EXT KVM register %s, error %d",
> +                             multi_ext_cfg->name, errno);


Same with the other 3 instances you changed in the patch. Thanks,


Daniel



>                   exit(EXIT_FAILURE);
>               }
>           } else {
> @@ -895,8 +894,8 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
>            *
>            * Error out if we get any other errno.
>            */
> -        error_report("Error when accessing get-reg-list, code: %s",
> -                     strerrorname_np(errno));
> +        error_report("Error when accessing get-reg-list: %s",
> +                     strerror(errno));
>           exit(EXIT_FAILURE);
>       }
>   
> @@ -905,8 +904,8 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
>       reglist->n = rl_struct.n;
>       ret = ioctl(kvmcpu->cpufd, KVM_GET_REG_LIST, reglist);
>       if (ret) {
> -        error_report("Error when reading KVM_GET_REG_LIST, code %s ",
> -                     strerrorname_np(errno));
> +        error_report("Error when reading KVM_GET_REG_LIST: %s",
> +                     strerror(errno));
>           exit(EXIT_FAILURE);
>       }
>   
> @@ -927,9 +926,8 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
>           reg.addr = (uint64_t)&val;
>           ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, &reg);
>           if (ret != 0) {
> -            error_report("Unable to read ISA_EXT KVM register %s, "
> -                         "error code: %s", multi_ext_cfg->name,
> -                         strerrorname_np(errno));
> +            error_report("Unable to read ISA_EXT KVM register %s: %s",
> +                         multi_ext_cfg->name, strerror(errno));
>               exit(EXIT_FAILURE);
>           }
>