[CRIU] [PATCH v3 1/3] Printout early log messages

[Dmitry Safonov]

Hi Adrian, Mike,

On 12/12/18 12:31 PM, Mike Rapoport wrote:
> Hi Adrian,
> 
> Sorry that I'm jumping this late, I've been a bit behind the CRIU list
> these days.
> 
> On Wed, Dec 12, 2018 at 07:44:50AM +0000, Adrian Reber wrote:
>> From: Adrian Reber <areber at redhat.com>
>>
>> Up until now any pr_* logging command (except pr_msg) was lost if
>> logging has not been set up (log_init()).
>>
>> To make sure not logging messages are lost, vprint_on_level() now writes
>> all messages (except LOG_MSG) to a temporary buffer which is written
>> to the logging FD once that is set up.
>>
>> Related: https://github.com/checkpoint-restore/criu/issues/578

I very much like the idea :)

>> Signed-off-by: Adrian Reber <areber at redhat.com>
>> ---
>>  criu/log.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 83 insertions(+), 1 deletion(-)

[..]

>>  
>>  	if (unlikely(loglevel == LOG_MSG)) {
>>  		fd = STDOUT_FILENO;
>>  		off = buf_off; /* skip dangling timestamp */
>>  	} else {
>> -		if (loglevel > current_loglevel)
>> +		/*
>> +		 * If logging has not yet been initialized (init_done == 0)
>> +		 * make sure all messages are written to the early_log_buffer.
>> +		 */
>> +		if ((loglevel > current_loglevel) && init_done)
> 
> I think a nicer way would be to have
> 
> 		if (!init_done) {
> 			early_vprint(format, params);
> 			return;
> 		}
> 		if (loglevel > current_loglevel)
> 			return;

Probably loglevel check should be before early print?

>>  			return;
>>  		fd = log_get_fd();
>>  		if (current_loglevel >= LOG_TIMESTAMP)
>>  			print_ts();
>>  	}
>>  
>> +	if (!init_done && !(loglevel == LOG_MSG)) {
> 
> and have the whole block below as
> 
> void early_vprint(const char *format, va_list params)
> 

Yes, please, it asks for a helper.


Thanks,
          Dima