> On Jan 31, 2017, at 7:35 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: > > Hi Mandy, > > Looks good to me in general. > > Suggestion: FileLoginModule.java > > 112 private static final String PASSWORD_FILE_NAME = "jmxremote.password"; > > Maybe the constant could be public, then it could be > referred to by ConnectorBootstrap? > I see that com.sun.jmx.remote.internal is already > being exported to jdk.management.agent. >
You meant com.sun.jmx.remote.security. I’d leave these 2 constants as is. Some future closer look to the relationship between FileLoginModule and ConnectorBootstrap may help to determine if future clean up should be done. > FileSystem.java: > > I wonder if this functionality should have remained in > java.management. It doesn't have any type to any specific > protocol - and it seems to be something that anyone using > the FileLoginModule above might also need. > That was added for the management agent and solely internal. We should look into converting the native code with NIO 2. No one should depend on it. > ConnectorBootstrap.java: > > I would normally discourage creating helper methods > (such as config()) in a class that doesn't implement > System.Logger, as that will make the 'config' method > appear as the source of the log message (rather than > the method that calls config()). > In this specific case I don't think it matters much > though. It's probably better to keep it like this so > as not distract reviewers from the meaningful changes, > and we can clean that up any time later in a separate > changeset (and possibly not before 10). > This is just an expedient way to remove the dependency to ClassLogger since the source method isn’t significant in this case. Thanks for the review. Mandy > > best regards, > > -- daniel > > On 30/01/17 23:48, Mandy Chung wrote: >> Webrev at: >> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173608/webrev.00/index.html >> >> Mandy >> >>> On Jan 30, 2017, at 3:42 PM, Mandy Chung <mandy.ch...@oracle.com> wrote: >>> >>> java.management is the module for JMX and JMX remote API and >>> java.lang.management. JDK management agent provides the JDK-specific >>> out-of-the-box monitoring and management support and it’s cleaner >>> for it to live in its own module. It is proposed to move it >>> to a new module named `jdk.management.agent`. >>> >>> This change involves: >>> 1) renaming of sun.management.Agent along with 3 other classes >>> in sun.management package to jdk.internal.agent package >>> >>> 2) move sun.management.resources to jdk.internal.agent.resources >>> >>> 3) move sun.management.jmxremote and sun.management.jdp packages >>> to jdk.management.agent module >>> >>> 4) move the configuration files under conf/management >>> >>> 5) sun/management/jmxremote/ConnectorBootstrap.java is updated >>> to replace the use of the ClassLogger API with System.Logger. >>> >>> 6) change hotspot VM to add `jdk.management.agent` when >>> -Dcom.sun.management.* system property is set as well as >>> the Agent class rename >>> >>> Daniel is working on JDK-8173607 [1] that conflicts with this >>> patch and require merges/coordination. >>> >>> We propose to integrate these changes to jdk9/dev unless >>> there is any objection concerning that this trivial hotspot change >>> might cause any issue. I have submitted a RBT on hotspot_runtime >>> and hotspot_serviceability in addition to PIT test runs. >>> >>> thanks >>> Mandy >>> [1] https://bugs.openjdk.java.net/browse/JDK-8173607 >> >
