On Tue, 13 Apr 2021 04:23:52 GMT, Koichi Sakata <ksak...@openjdk.org> wrote:
> When reconfiguring log output options of an existing log setting with jcmd > VM.log and the file name, a warning message is shown. However, when > reconfiguring them with jcmd VM.log and the output index, the message is not > shown. > > > $ java -Xlog::sample.log:: Sample > $ jcmd 4976 VM.log list > Log output configuration: > #0: stdout all=warning uptime,level,tags > #1: stderr all=off uptime,level,tags > #2: file=sample.log all=info uptime,level,tags filecount=5,filesize=20480K > > $ jcmd 4976 VM.log output="sample.log" output_options="filecount=100" > 4976: > Output options for existing outputs are ignored. > > $ jcmd 4976 VM.log output="#2" output_options="filecount=100" > 4976: > Command executed successfully > > > It's better to show the same message also in the latter case, I think. > Because the output options is not updated in both cases. > > > $ jcmd 4976 VM.log list > #0: stdout all=warning uptime,level,tags > #1: stderr all=off uptime,level,tags > #2: file=sample.log all=info uptime,level,tags filecount=5,filesize=20480K > (reconfigured) > > This pull request enable to show the warning message when specifying the > output index. > > > $ java -Xlog::sample.log:: Sample > $ jcmd 5054 VM.log output="sample.log" output_options="filecount=100" > 5054: > Output options for existing outputs are ignored. > > $ jcmd 5054 VM.log output="#2" output_options="filecount=100" > 5054: > Output options for existing outputs are ignored. > > > ### gtest result > > > $ > ./build/linux-x86_64-server-fastdebug/hotspot/variant-server/libjvm/gtest/gtestLauncher > -jdk:./build/linux-x86_64-server-fastdebug/jdk > --gtest_filter=LogConfigurationTest.* > Note: Google Test filter = LogConfigurationTest.* > [==========] Running 15 tests from 1 test case. > [----------] Global test environment set-up. > [----------] 15 tests from LogConfigurationTest > [ RUN ] LogConfigurationTest.describe_vm > [ OK ] LogConfigurationTest.describe_vm (1 ms) > [ RUN ] LogConfigurationTest.update_output_vm > [ OK ] LogConfigurationTest.update_output_vm (0 ms) > [ RUN ] LogConfigurationTest.add_new_output_vm > [ OK ] LogConfigurationTest.add_new_output_vm (0 ms) > [ RUN ] LogConfigurationTest.disable_logging_vm > [ OK ] LogConfigurationTest.disable_logging_vm (1 ms) > [ RUN ] LogConfigurationTest.disable_output_vm > [ OK ] LogConfigurationTest.disable_output_vm (0 ms) > [ RUN ] LogConfigurationTest.reconfigure_decorators_vm > [ OK ] LogConfigurationTest.reconfigure_decorators_vm (0 ms) > [ RUN ] LogConfigurationTest.invalid_configure_options_vm > [ OK ] LogConfigurationTest.invalid_configure_options_vm (0 ms) > [ RUN ] LogConfigurationTest.parse_empty_command_line_arguments_vm > [0.462s][warning][logging] Ignoring excess -Xlog options: "" > [ OK ] LogConfigurationTest.parse_empty_command_line_arguments_vm (0 ms) > [ RUN ] LogConfigurationTest.parse_command_line_arguments_vm > [ OK ] LogConfigurationTest.parse_command_line_arguments_vm (1 ms) > [ RUN ] LogConfigurationTest.parse_log_arguments_vm > [ OK ] LogConfigurationTest.parse_log_arguments_vm (188 ms) > [ RUN ] LogConfigurationTest.configure_stdout_vm > [ OK ] LogConfigurationTest.configure_stdout_vm (2 ms) > [ RUN ] LogConfigurationTest.subscribe_vm > [ OK ] LogConfigurationTest.subscribe_vm (2 ms) > [ RUN ] LogConfigurationTest.parse_invalid_tagset_vm > [ OK ] LogConfigurationTest.parse_invalid_tagset_vm (0 ms) > [ RUN ] LogConfigurationTest.output_name_normalization_vm > [ OK ] LogConfigurationTest.output_name_normalization_vm (1 ms) > [ RUN ] LogConfigurationTest.suggest_similar_selection_vm > [ OK ] LogConfigurationTest.suggest_similar_selection_vm (1 ms) > [----------] 15 tests from LogConfigurationTest (658 ms total) > > [----------] Global test environment tear-down > [==========] 15 tests from 1 test case ran. (659 ms total) > [ PASSED ] 15 tests. Sorry for the delay in noticing this PR! Fix looks good. One nit on variable naming below. Thanks, David src/hotspot/share/logging/logConfiguration.cpp line 430: > 428: ConfigurationLock cl; > 429: size_t idx; > 430: bool is_added = false; Nit: `was_added` (or even just `added`) ------------- Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3448
