Hi,

Please find below the revised webrev.

http://cr.openjdk.java.net/~hb/8187498/webrev.02/


On Friday 09 February 2018 05:07 AM, mandy chung wrote:
On 2/7/18 1:19 AM, Harsha Wardhana B wrote:
> > --start-management-agent will not be recognized in the current format and
> hence will not default to --start-management-agent=local=true.

`--start-local-management-server` is one option as Alan suggests.
Another option is to make `--start-management-agent` to accept
an optional argument.  VM can accept `--start-management-agent`
or `--start-management-agent=port=1234,ssl=on`.  It may require
more launcher change to support it.
Yes. It requires lot more changes to launcher. Hence optional argument to --start-management-agent was not added in order to keep the launcher impact minimum.
> Below is the webrev with above changes and corresponding tests.
>
> http://cr.openjdk.java.net/~hb/8187498/webrev.01/

arguments.cpp

3216         if (FLAG_SET_CMDLINE(bool, ManagementServer, true) != 
Flag::SUCCESS) {
3217           return JNI_EINVAL;
3218         }
3219         // management agent in module jdk.management.agent
3220         if (!create_numbered_property("jdk.module.addmods", 
"jdk.management.agent", addmods_count++)) {
3221           return JNI_ENOMEM;
3222         }

- you can consider refactor this to replace this block and line 2988-3994.

3224         jio_fprintf(defaultStream::output_stream(),
3225           "-Xmanagement is not supported in this VM.\n");

-Xmanagement not yet renamed
java.c
   To set the precedence for future VM long form options, I suggest to
rename IsManagementOption to IsVMLongFormOption and change the error
message not specific to management agent.
Incorporated above changes.
Agent.java
  If --start-management-agent is set, -Dcom.sun.management.* takes
  precedence.  Do you really want to do that?  The new VM option
  intends to simplify the command-line to type in.  I think it's
  cleaner and reasonable if --start-management-agent is specified,
  -Dcom.sun.management.* are ignored (maybe worth emit a warning if set)
We can probably document that -D options will be overridden instead of emitting a warning.
  The implementation uses VMManagement::getVmArguments and scan
  the VM options for -start-management-agent.  The VM is the one
  invoking jdk.internal.agent.Agent::startAgent.  A simpler option
  is to change Agent::startAgent to take an argument parameter
  that will be passed with the argument of --start-management-agent
  or null if not set.

  Similarly for startRemoteManagementAgent and startLocalAgent_name.
Implementing this change requires lot of changes to argument parsing in native code and hence it is simpler to handle this at java layer.
  parseXmgmtArgs should be renamed as no longer -Xmgmt. Better not
  to tightly coupled with the option name.
Ok.
  291         if(args.startsWith("--start-management-agent")) {
  292             return parseXmgmtArgs(args);
  293         }

What is this change for?
This is required when management agent is started via attach mechanism.
Mandy
-Harsha

Reply via email to