[crac] RFR: Support updating MANAGEABLE JVM options during restore [v2]

[Anton Kozlov]

On Wed, 3 May 2023 09:45:09 GMT, Radim Vansa <duke at openjdk.org> wrote:

>> When a JVM option is MANAGEABLE it can be set at any time during runtime, therefore it is safe to change it during the restore operation. Rather than silently ignoring JVM options passed along with -XX:CRaCRestoreFrom we send them to the restored process and either update or print a warning if given option cannot be changed.
>
> Radim Vansa has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Use RESTORE_SETTABLE on JVM flags
>   
>   * Fail early when using non settable flags
>   * CracBuilder fix: don't use classpath during restore

Thanks, looks mostly good except few nits.

Do you have an idea why jdk/crac/recursiveCheckpoint/Test.java has failed?

src/hotspot/share/runtime/globals.hpp line 57:

> 55: 
> 56: // The optional extra_attrs parameter may have one of the following values:
> 57: // DIAGNOSTIC, EXPERIMENTAL, MANAGEABLE and RESTORE_SETTABLE. Currently

` and` -> `, or`

src/hotspot/share/runtime/globals.hpp line 2094:

> 2092:                 "Trace optimized upcall stub generation")                   \
> 2093:                                                                             \
> 2094:   product(ccstr, CRaCCheckpointTo, NULL, MANAGEABLE,                        \

I see reasons for CRaCCheckpointTo to be MANAGEABLE, but at the moment the flag is assumed to be set in the command line by the implementation, e.g. os::Linux::{vm_create_start,prepare_checkpoint} are called depending on the flag value, and that can happen only during VM initialization.

A set of changes are required before the option can become MANAGEABLE.

The test should also updated once the option is reverted.

src/hotspot/share/runtime/globals.hpp line 2129:

> 2127:       "Throw CheckpointException if uncheckpointable resource handle found")\
> 2128:                                                                             \
> 2129:   product(bool, CRTrace, true, MANAGEABLE, "Minimal C/R tracing")           \

RESTORE_SETTABLE was meant here? Please don't mix in MANAGEABLE flags into this PR if that was inteded.

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

PR Comment: https://git.openjdk.org/crac/pull/61#issuecomment-1532862561
PR Comment: https://git.openjdk.org/crac/pull/61#issuecomment-1532864110
PR Review Comment: https://git.openjdk.org/crac/pull/61#discussion_r1183538871
PR Review Comment: https://git.openjdk.org/crac/pull/61#discussion_r1183553692
PR Review Comment: https://git.openjdk.org/crac/pull/61#discussion_r1183485336