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

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

On 12/18/23 18:35, Michael Tokarev wrote:
> 18.12.2023 20:20, Daniel Henrique Barboza wrote:
>>
>>
>> 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
> 
> If KVM context puts its own unique meaning for ENOENT, maybe something like
> 
>   "unable to read KVM register: %s\n", errno == ENOENT ? "no such register" : strerror(errno)
> 
> would do it better?


A solution like this is something I can go after if I'm bothered enough with how strerror()
is working in the RISC-V KVM driver.

For now I think we can live with this fix as is since fixing the build is more important
that aesthetics.


Thanks,

Daniel

> 
> To me, "No such file or directory" already tells everything and does not look
> weird, but that's because I've seen this error message for all sorts of contexts
> and got used to this. It is definitely understandable.
> 
> /mjt