RFR: 8196729: Add jstatd option to specify RMI connector port

[serguei.spitsyn at oracle.com]

Hi Daniil,

Looks good.
Thank you for the update!

Thanks,
Serguei


On 2/4/20 22:00, Daniil Titov wrote:
> Hi Serguei,
>
> Thank you for finding this! Please review the new version of webrev [1]
> that has it corrected. The new webrev also includes changes in the test to
> make sure that all jstatd tests run for both styles of command line options.
>   
> Testing: Mach5 jobs for sun/tools/jstatd  succeeded.  Tiers1, tiers2, tiers3,
> and tiers5 job are in the progress.
>
> [1] Webrev:  http://cr.openjdk.java.net/~dtitov/8196729/webrev.02/
> [2] Jira issue: https://bugs.openjdk.java.net/browse/JDK-8196729
> [3] CSR : https://bugs.openjdk.java.net/browse/JDK-8238357
>
> Thanks,
> Daniil
>
> From: "serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com>
> Date: Tuesday, February 4, 2020 at 7:51 PM
> To: Daniil Titov <daniil.x.titov at oracle.com>, "serviceability-dev at openjdk.java.net" <serviceability-dev at openjdk.java.net>
> Subject: Re: RFR: 8196729: Add jstatd option to specify RMI connector port
>
> Hi Daniil,
>
> It looks okay to me in general.
> But I'm puzzled with this part:
>
> http://cr.openjdk.java.net/~dtitov/8196729/webrev.01/src/jdk.jstatd/share/classes/sun/tools/jstatd/Jstatd.java.udiff.html
> +            } else if (arg.startsWith("-r")) {
> +                if (arg.compareTo("-r") != 0) {
> +                    port = Integer.parseInt(arg.substring(2));
> +                } else {
> +                    argc++;
> +                    if (argc >= args.length) {
> +                        printUsage();
> +                        System.exit(1);
> +                    }
> +                    rmiPort = Integer.parseInt(args[argc]);
> +                }
>
> The option -r is for rmi connection port number.
> Why does this code set the RMI registry port? :
> +                if (arg.compareTo("-r") != 0) {
> +                    port = Integer.parseInt(arg.substring(2));
>
> Thanks,
> Serguei
>
>
> On 1/31/20 13:08, Daniil Titov wrote:
> Please review change [1] that adds a new command line option to jstatd tool to specify a RMI connector port.
>
> Currently a random port is used that prevents this tool from being used behind a firewall or in a container.
>
> New CSR [3] was created for this change and it needs to be reviewed as well.
>
> Man pages for jstatd will be updated in a separate issue.
>
> Testing: Mach5 tier1-tier3 and sun/tools/jstatd/* tests  succeeded. Mach5 tier5 tests are in progress.
>   
> [1] Webrev: http://cr.openjdk.java.net/~dtitov/8196729/webrev.01/
> [2] Jira issue: https://bugs.openjdk.java.net/browse/JDK-8196729
> [3] CSR : https://bugs.openjdk.java.net/browse/JDK-8238357
>
> Thank you,
> Daniil
>
>
>
>
>
>
>