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

[Daniel Henrique Barboza]

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