On 9 jun 2014, at 21:54, Alan Bateman <alan.bate...@oracle.com> wrote:
> On 09/06/2014 20:03, Staffan Larsen wrote: >> This is the first part in a two-part series of removing the >> management-agent.jar and replacing its functionality with APIs in the attach >> framework. In this change I have added the new APIs, a later change will >> remove management-api.jar. >> >> management-agent.jar is the java agent that is used with the attach API to >> start the JMX agent in a target VM. It's the approach used in JDK 6 to start >> JMX in a running VM and predates the "jcmd ManagementAgent.start" command >> added in 7uX. >> management-agent.jar will be problematic when we move to a modular JDK in >> JDK 9 and should be replaced by a "real" API. So this change adds two >> methods to VirtualMachine in the attach framework for starting either a >> local or a remote management agent. >> >> webrev: http://cr.openjdk.java.net/~sla/8044135/webrev.00/ >> bug: https://bugs.openjdk.java.net/browse/JDK-8044135 >> > This looks reasonable to me. I assume that the removal of > management-agent.jar will come in another change and that tests such as > LocalManagementTest will be updated then. That was my plan, but now I think it would be better to update the tests in this change to validate that the new API works as expected by the tests. > On this webrev then there are few unnecessary imports in > HotSpotVirtualMachine - I only noticed these as it initially looked like this > was introducing a dependency on JMX. Fixed. > IllegalArgumentException isn't a checked exception so there isn't a need for > verifyKeyName to declare it. Missing space in "if(" at line 174. An > alternative name for verifyKeyName is checkedKeyName - might be more natural > when used with filter. Fixed. Thanks, /Staffan > > -Alan.