Re: [Qemu-devel] [PATCH 14/14] usb: Proper error propagation for usb_device_attach errors

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 05/31/2011 12:12 PM, Kevin Wolf wrote:
> Am 31.05.2011 12:05, schrieb Hans de Goede:
>> Hi,
>>
>> On 05/31/2011 11:56 AM, Kevin Wolf wrote:
>>> Am 31.05.2011 11:51, schrieb Hans de Goede:
>>>> Hi,
>>>>
>>>> On 05/31/2011 11:42 AM, Michael Tokarev wrote:
>>>>> 31.05.2011 13:35, Hans de Goede wrote:
>>>>>> ---
>>>>>>     hw/usb-bus.c |   23 ++++++++++++-----------
>>>>>>     hw/usb-msd.c |    5 +++--
>>>>>>     usb-linux.c  |    6 +++++-
>>>>>>     3 files changed, 20 insertions(+), 14 deletions(-)
>>>>>>
>>>>>> diff --git a/hw/usb-bus.c b/hw/usb-bus.c
>>>>>> index 0a49921..2ae2678 100644
>>>>>> --- a/hw/usb-bus.c
>>>>>> +++ b/hw/usb-bus.c
>>>>>
>>>>>>         if (dev->attached) {
>>>>>> -        fprintf(stderr, "Warning: tried to attach usb device %s twice\n",
>>>>>> +        fprintf(stderr, "Error: tried to attach usb device %s twice\n",
>>>>>>                     dev->product_desc);
>>>>>
>>>>> qemu_error() maybe, while we're at it?
>>>>> Here and in a few other places.
>>>>
>>>> That does not seem to exist, do you perhaps mean error_printf() ?
>>>
>>> error_report() is what you should use, so that messages go to the
>>> monitor if the function is called from a monitor command. error_printf()
>>> is used by it internally, but usually isn't used directly.
>>>
>>
>> I've looked at error_report, but IMHO it is made of crazy, I'm not going
>> to construct a json dict every time I need to log some simple error message
>> (and the existing ones are not suitable for many error messages).
>
> error_report() works with plain strings. Maybe you confuse it with the
> QMP error reporting function?

Ah yes I was looking at qerror_report (who ever named that, having just
one letter difference in the function names is a bad idea). error_report
looks fine.

I'll wait a bit for more feedback and then change
[PATCH 14/14] usb: Proper error propagation for usb_device_attach errors

To turn the fprintf(stderr, ... calls into error_report calls.

Thanks & Regards,

Hans



>
> Kevin
>