RFR(xs) 8186988: use log_warning() and log_error() instead of tty->print_cr for CDS warning and error messages

[David Holmes]

Hi Calvin,

On 12/09/2019 4:53 am, Ioi Lam wrote:
> I agree that as long as the message itself it unchanged, having the 
> extra UL decoration should be OK.

I would agree with that too.

Given there are already uses of log_error and log_warning for CDS 
messages, converting the remaining tty uses seems appropriate. Though as 
always with UL I find it hard to clearly identify the appropriate tags 
to use e.g. just cds, or cds+X for some x.

> For the "Preload Warning", I think it's prudent to keep it. I withdraw 
> my request to remove it.

I keep flipping between agreeing and not agreeing with this - why should 
this case be different? I could argue that anyone doing -Xshare:dump is 
very unlikely to explicitly configure UL at the same time and so by 
default we would see all warnings and errors on stdout, just as now. But 
on the other hand when all the VM is doing is dumping and terminating 
you may not want to risk losing important warnings just because the user 
has UL "mis-configured". Your call. :)

Thanks,
David
-----

> Thanks
> - Ioi
> 
> 
> On 9/11/2019 11:25 AM, Calvin Cheung wrote:
>> Hi Thomas,
>>
>> Thanks for your review.
>>
>> On 9/11/19 10:41 AM, Thomas Stüfe wrote:
>>> Hi Calvin,
>>>
>>> looks okay. This will cause the messages to be printed with UL 
>>> decorations, right?
>>
>> Yes, after the change, an error message would look like:
>>
>> [0.337s][error][cds] Error: non-empty directory 
>> '/scratch/jtreg_test/JTwork/scratch/0/mods/com.simple/
>>
>> Before change:
>>
>> Error: non-empty directory 
>> '/scratch/jtreg_test/JTwork/scratch/21/mods/com.simple/
>>
>>> Is that okay or could this cause backward compatibility problems?
>>
>> Probably okay as long as the message stays the same.
>>
>> thanks,
>>
>> Calvin
>>
>>>
>>> Cheers, Thomas
>>>
>>> On Wed, Sep 11, 2019 at 6:29 PM Calvin Cheung 
>>> <calvin.cheung at oracle.com <mailto:calvin.cheung at oracle.com>> wrote:
>>>
>>>     bug: https://bugs.openjdk.java.net/browse/JDK-8186988
>>>
>>>     webrev: http://cr.openjdk.java.net/~ccheung/8186988/webrev.00/
>>>
>>>     Please review this simple change for replacing the use of
>>>     tty->print_cr
>>>     with log_waring(cds) for CDS warning messages and with 
>>> log_error(cds)
>>>     for error messages.
>>>
>>>     Ran tier1 - 3 testing.
>>>
>>>     thanks,
>>>
>>>     Calvin
>>>
>