Re: [Qemu-devel] [PATCH 1/2] qemu-error: Introduce get_errno_name()

[Anthony Liguori <anthony@codemonkey.ws>]

On 05/03/2010 08:06 AM, Markus Armbruster wrote:
> Luiz Capitulino<lcapitulino@redhat.com>  writes:
>
>    
>> We need to expose errno in QMP, for three reasons:
>>
>>    1. Some error handling functions print errno codes to the user,
>>       while it's debatable whether this is good or not from a user
>>       perspective, sometimes it's the best we can do because it's
>>       what system calls and libraries return
>>
>>    2. Some events (eg. BLOCK_IO_ERROR) will be made even more
>>       complete with errno information
>>
>>    3. It's very good for debugging
>>
>> So, we need a way to expose those codes in QMP. We can't just use
>> the codes themselfs because they may vary between systems.
>>
>> The best solution I can think of is to return the string
>> representation of the name. For example, EIO becomes "EIO".
>>
>> This is what get_errno_name() does.
>>
>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
>> ---
>>   qemu-error.c |   85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   qemu-error.h |    1 +
>>   2 files changed, 86 insertions(+), 0 deletions(-)
>>
>> diff --git a/qemu-error.c b/qemu-error.c
>> index 5a35e7c..7035417 100644
>> --- a/qemu-error.c
>> +++ b/qemu-error.c
>> @@ -207,3 +207,88 @@ void error_report(const char *fmt, ...)
>>       va_end(ap);
>>       error_printf("\n");
>>   }
>> +
>> +/*
>> + * Probably only useful for QMP
>> + */
>> +const char *get_errno_name(int err)
>> +{
>> +    switch (abs(err)) {
>> +    case EPERM:
>> +        return "EPERM";
>> +    case ENOENT:
>> +        return "ENOENT";
>>      
> [...]
>    
>> +    case EDOM:
>> +        return "EDOM";
>> +    case ERANGE:
>> +        return "ERANGE";
>> +    case ENOMEDIUM:
>> +        return "ENOMEDIUM";
>> +    case ENOTSUP:
>> +        return "ENOTSUP";
>> +    default:
>> +        return "unknown";
>>      
> How did you choose the codes to implement?  POSIX has many more...
>    

Let me say another way why I think this is a bad path to go down.

In generally, we could never just pass errno through down.  Different 
host platforms are going to generate different errno values so we really 
need to filter and send reliable errno values so that clients don't have 
to have special code for when they're on Linux vs. AIX vs. Solaris.

If we're white listing errno values, we should be able to trivially 
convert errnos to QError types via a table just like you have above.

Regards,

Anthony Liguori

>> +    }
>> +
>> +    abort();
>> +}
>> diff --git a/qemu-error.h b/qemu-error.h
>> index a45609f..c5e39b5 100644
>> --- a/qemu-error.h
>> +++ b/qemu-error.h
>> @@ -38,4 +38,5 @@ void error_print_loc(void);
>>   void error_set_progname(const char *argv0);
>>   void error_report(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
>>
>> +const char *get_errno_name(int err);
>>   #endif
>>      
>
>