Re: [Qemu-devel] [PATCH] ide: log error when trying to use ATAPI overlapping features

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> Am 11.02.2013 14:27, schrieb Stefan Hajnoczi:
>> On Sun, Feb 10, 2013 at 11:12:11PM +0100, Hervé Poussineau wrote:
>>>
>>> Signed-off-by: Hervé Poussineau <address@hidden>
>>> ---
>>>  hw/ide/core.c |    4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>>> index 3743dc3..f0ab1a8 100644
>>> --- a/hw/ide/core.c
>>> +++ b/hw/ide/core.c
>>> @@ -1394,8 +1394,10 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>>>          break;
>>>      case WIN_PACKETCMD:
>>>          /* overlapping commands not supported */
>>> -        if (s->feature & 0x02)
>>> +        if (s->feature & 0x02) {
>>> +            qemu_log_mask(LOG_UNIMP, "ide: overlapping command not 
>>> supported");
>> 
>> qemu_log_*() isn't used much in hw/.
>> 
>> I think we need to side-track this patch email to figure out what to
>> use:
>> 
>> fprintf(stderr) - some warnings/errors use this
>> error_report() - goes to the monitor, if possible, otherwise stderr
>
> These look wrong to me.

"Wrong" is a bit strong, in particular since there's ample precedence
for these uses.

>> qemu_log_*() - goes to the qemu log, seems a little TCG-centric
>
> I would suggest either this or just trace points. (And by the way, it's
> a pity that -d is so TCG-centric, it's been more than once the reason
> why I disabled KVM when debugging a guest... Having at least -d int
> would be so useful.)

Tracepoints don't really fit when we want to report the guest does
something we don't handle.  Users deserve fair warning then, don't they?

Could qemu_log() & friends be made fit for general use?  What's missing?