[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hw/audio/sb16.c: Convert file to new logging AP
From: |
Programmingkid |
Subject: |
Re: [Qemu-devel] [PATCH] hw/audio/sb16.c: Convert file to new logging API |
Date: |
Wed, 31 Jan 2018 17:18:40 -0500 |
> On Jan 31, 2018, at 4:43 PM, Programmingkid <address@hidden> wrote:
>
>>
>> On Jan 31, 2018, at 4:22 AM, Gerd Hoffmann <address@hidden> wrote:
>>
>> On Tue, Jan 30, 2018 at 10:30:46AM -0500, Programmingkid wrote:
>>>
>>>> On Jan 30, 2018, at 4:41 AM, Gerd Hoffmann <address@hidden> wrote:
>>>>
>>>>> @@ -148,15 +142,16 @@ static int irq_of_magic (int magic)
>>>>> #if 0
>>>>> static void log_dsp (SB16State *dsp)
>>>>> {
>>>>> - ldebug ("%s:%s:%d:%s:dmasize=%d:freq=%d:const=%d:speaker=%d\n",
>>>>> - dsp->fmt_stereo ? "Stereo" : "Mono",
>>>>> - dsp->fmt_signed ? "Signed" : "Unsigned",
>>>>> - dsp->fmt_bits,
>>>>> - dsp->dma_auto ? "Auto" : "Single",
>>>>> - dsp->block_size,
>>>>> - dsp->freq,
>>>>> - dsp->time_const,
>>>>> - dsp->speaker);
>>>>> + qemu_log_mask(LOG_UNIMP, "%s:%s:%d:%s:dmasize=%d:freq=%d:const=%d:"
>>>>> + "speaker=%d\n",
>>>>> + dsp->fmt_stereo ? "Stereo" : "Mono",
>>>>> + dsp->fmt_signed ? "Signed" : "Unsigned",
>>>>> + dsp->fmt_bits,
>>>>> + dsp->dma_auto ? "Auto" : "Single",
>>>>> + dsp->block_size,
>>>>> + dsp->freq,
>>>>> + dsp->time_const,
>>>>> + dsp->speaker);
>>>>> }
>>>>> #endif
>>>>
>>>> Hmm, dead code. Any places which call log_dsp() ?
>>>
>>> There are several places but they are all dead code. Do you want this
>>> removed?
>>
>> So it is still used but not compiled in by default.
>>
>> Looking at all of this again, I think "qemu_log_mask(LOG_UNIMP, ...)"
>> should only be used in case there *really* is some soundblaster feature
>> unimplemented. Otherwise converting to a tracepoint is probably more
>> useful, so the debug messages can be enabled/disabled individually at
>> runtime.
>>
>> The "#if 0" seems to be there to limit the flood of debug messages. So
>> once they all are converted to trace points this is not needed any more
>> and I think we should just enable all this code.
>>
>> dolog() seems to be mostly cases where the code warns about
>> unimplemented featues or incomplete emulation, so using LOG_UNIMP looks
>> sensible.
>
> Simply replacing dolog() with "qemu_log_mask(LOG_UNIMP," seems like what was
> desired.
>
>> ldebug() should probably trace points.
>
> I need help with this. I tried replacing ldebug() with
> "qemu_log_mask(LOG_TRACE,", but that caused a lot of messages to be printed
> to the terminal. It is odd because I thought I had to use "-d trace" to make
> it print. Do you have another suggestion on what to replace ldebug() with?
>
>> Probably makes sense to split this patch up into a small series.
>
> The first patch will be the replacing dolog() with qemu_log_mask(LOG_UNIMP.
> The second patch will be replacing ldebug() with what you suggest.
After doing a little more research I found this code in log.c:
/* enable or disable low levels log */
void qemu_set_log(int log_flags)
{
qemu_loglevel = log_flags;
#ifdef CONFIG_TRACE_LOG
qemu_loglevel |= LOG_TRACE;
#endif
It looks like no matter what the trace level that is set, LOG_TRACE will always
be used. I think this might be a mistake. The configure script will set
CONFIG_TRACE_LOG if the backend is log. This appears to be the default
behavior. This will lead to LOG_TRACE being set in util/log.c.