Re: [Qemu-devel] [PATCH 07/22] qemu-error: Introduce get_errno_string()

[Markus Armbruster]

"Daniel P. Berrange" <address@hidden> writes:

> On Wed, Apr 21, 2010 at 12:12:14PM -0300, Luiz Capitulino wrote:
>> On Wed, 21 Apr 2010 18:42:38 +0400 (MSD)
>> malc <address@hidden> wrote:
>> 
>> > On Wed, 21 Apr 2010, Kevin Wolf wrote:
>> > 
>> > > Am 21.04.2010 10:28, schrieb Daniel P. Berrange:
>> > > > On Tue, Apr 20, 2010 at 06:09:37PM -0300, Luiz Capitulino wrote:
>> > > >> There are error handling functions in QEMU which print errno codes
>> > > >> to the user. While it's debatable if this is good from a user
>> > > >> perspective, sometimes it's the best you can do because it's what
>> > > >> system calls return and this is also useful for debugging.
>> > > >>
>> > > >> So, we need a way to expose those codes in QMP. We can't use the
>> > > >> codes themselfs because they may vary between systems.
>> > > >>
>> > > >> The best solution I can think of is returning the string
>> > > >> representation of the name. For example, EIO becomes "EIO".
>> > > >>
>> > > >> This is what get_errno_string() does.
>> > > >>
>> > > >> Signed-off-by: Luiz Capitulino <address@hidden>
>> > > >> ---
>> > > >>  qemu-error.c |   25 +++++++++++++++++++++++++
>> > > >>  qemu-error.h |    1 +
>> > > >>  2 files changed, 26 insertions(+), 0 deletions(-)
>> > > >>
>> > > >> diff --git a/qemu-error.c b/qemu-error.c
>> > > >> index 5a35e7c..55ce133 100644
>> > > >> --- a/qemu-error.c
>> > > >> +++ b/qemu-error.c
>> > > >> @@ -207,3 +207,28 @@ void error_report(const char *fmt, ...)
>> > > >>      va_end(ap);
>> > > >>      error_printf("\n");
>> > > >>  }
>> > > >> +
>> > > >> +/*
>> > > >> + * This is probably only useful for QMP
>> > > >> + */
>> > > >> +const char *get_errno_string(int err)
>> > > >> +{
>> > > >> +    assert(err < 0);
>> > > >> +
>> > > >> +    switch (err) {
>> > > >> +    case -EINVAL:
>> > > >> +        return "EINVAL";
>> > > >> +    case -EIO:
>> > > >> +        return "EIO";
>> > > >> +    case -ENOENT:
>> > > >> +        return "ENOENT";
>> > > >> +    case -ENOMEDIUM:
>> > > >> +        return "ENOMEDIUM";
>> > > >> +    case -ENOTSUP:
>> > > >> +        return "ENOTSUP";
>> > > >> +    default:
>> > > >> +        return "unknown";
>> > > >> +    }
>> > > >> +
>> > > >> +    abort();
>> > > >> +}
>> > > > 
>> > > > Wouldn't it be nicer to return strerror_r()  output instead of errno
>> > > > names ?
>> > > 
>> > > I agree. And it would be more complete, too.
>> > 
>> > OTOH it has a problem of returning translated messages (subject to
>> > LC_MESSAGES value).
>> 
>>  Exactly, and I'm not sure if there's anything that ensure they're
>> exactly the same among different systems.
>
> I thought QMP already declared that the printable error strings are subject
> to arbitrary change at any time, which includes translation? Apps needing 
> something reliable should be hooking onto the error code.

Yes, but the value of get_errno_string() is put into the error's data
object, where the "client should not attempt to parse this" clause does
not apply.

We need to decide whether clients need to know the errno or not.

If they do, we need to encode errno in a way that doesn't depend on
QEMU's host system.  The encoding proposed by Luiz is as good as any, I
think.

If they don't, then substituting text obtained from strerror_r() into
desc is the way to go.  There's currently no way to do that without
putting something it the error's data object, so that would need fixing.
Simple: don't send members whose names start with '_' across the wire.