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.

Reply via email to