RFR: 8256008: UL does not report anything if disk writing fails [v2]

[Thomas Stuefe]

On Thu, 19 Nov 2020 05:26:13 GMT, Yasumasa Suenaga <ysuenaga at openjdk.org> wrote:

>> If there are no disk space enough to write UL, it does not report. So the user cannot know lack of logs timely.
>> 
>> `LogFileStreamOutput::write()` uses `jio_fprintf()` and `fflush()` to write log. However return values from them would not be checked.
>> We should check them, and should report what happened if error occurred.
>> 
>> 
>> How to reproduce on Linux:
>> 
>> 1. Mount tmpfs with 512K
>> 
>> $ mkdir /tmp/tmp
>> $ mount -t tmpfs -o size=512k tmpfs /tmp/tmp
>> 
>> 2. Run `java` with `-Xlog`
>> 
>> $ java -Xlog:all=trace:file=/tmp/tmp/all.log --version
>
> Yasumasa Suenaga has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits:
> 
>  - refactoring
>  - Merge branch 'master' into JDK-8256008
>  - Fix typo
>  - 8256008: UL does not report anything if disk writing fails

Hi Yasumasa,

this is good, but some nits remain, see below.

Cheers, Thomas

src/hotspot/share/logging/logFileStreamOutput.cpp line 90:

> 88: };
> 89: 
> 90: static bool write_error_is_shown = false;

Can you make this a static member of LogFileStreamOutput please?

src/hotspot/share/logging/logFileStreamOutput.cpp line 106:

> 104: }
> 105: 
> 106: #define WRITE_LOG_WITH_RESULT_CHECK(op, total)                \

You may leave out total and reference "written" directly in the macro since its always used with "written". Leave it up to you though, its a matter of taste.

src/hotspot/share/logging/logFileStreamOutput.cpp line 109:

> 107: {                                                             \
> 108:   int result = op;                                            \
> 109:   if (result <= 0) {                                          \

May be not completely correct for all values of op:
- fprintf returns a negative value (0 is valid if length to write was 0). I did not check jio_fprintf but I assume the return semantics are the same?
- write_decorations returns 0 if it did not write any decorations, which is valid

-------------

Changes requested by stuefe (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1106