RFR : 8217786: Provide virtualization related info in the hs_error file on linux s390x

Tue Jan 29 16:56:44 UTC 2019

Hello, I added a break to avoid potential printing lines multiple times, and removed the comment line :

http://cr.openjdk.java.net/~mbaesken/webrevs/8217786.3/

Best regards, Matthias

From: Thomas Stüfe <thomas.stuefe at gmail.com>
Sent: Dienstag, 29. Januar 2019 15:22
To: Baesken, Matthias <matthias.baesken at sap.com>
Cc: David Holmes <david.holmes at oracle.com>; hotspot-dev at openjdk.java.net
Subject: Re: RFR : 8217786: Provide virtualization related info in the hs_error file on linux s390x

Hi Matthias,
+     if (strncmp(line, keywords_to_match[i], strlen(keywords_to_match[i])) == 0) {
+       st->print("%s", line);
+     }

you should break here otherwise a line containing multiple keywords will be printed multiple times.

+    // the LPAR / CPUs / VM - related infos usually come in blocks

This comment can be removed.

+#if defined(S390)

I still do not like the arch specific code here, but for now I can live with it. Should this section grow and cover other architectures as well, we should fan out into os_linux_<cpu>.cpp.

Thanks, Thomas

On Tue, Jan 29, 2019 at 12:22 PM Baesken, Matthias <matthias.baesken at sap.com<mailto:matthias.baesken at sap.com>> wrote:
Hello  here is a 2nd webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8217786.2/

  *   Introduced    static bool print_matching_lines_from_sysinfo_file(outputStream* st, const char* keywords_to_match[])
  *   Moved  call to Linux-only  print_os_info

Best regards, Matthias

From: Thomas Stüfe <thomas.stuefe at gmail.com<mailto:thomas.stuefe at gmail.com>>
Sent: Dienstag, 29. Januar 2019 09:23
To: Baesken, Matthias <matthias.baesken at sap.com<mailto:matthias.baesken at sap.com>>; David Holmes <david.holmes at oracle.com<mailto:david.holmes at oracle.com>>
Cc: hotspot-dev at openjdk.java.net<mailto:hotspot-dev at openjdk.java.net>
Subject: Re: RFR : 8217786: Provide virtualization related info in the hs_error file on linux s390x

I'm still unhappy with that solution, since we have fanned out this coding for all architectures into the architecture independent os_linux.cpp. A generic "Show matching lines from given file" would be a better (slimmer, better reusable) solution IMHO.

Side note: Could you please exchange strstr() .. with strncmp() since you require the start of the string to match. So no reason to parse the whole line if the start does not match.

Cheers, Thomas

On Tue, Jan 29, 2019 at 9:03 AM Baesken, Matthias <matthias.baesken at sap.com<mailto:matthias.baesken at sap.com>> wrote:

>
> No I was thinking more about just adding the virtualization info to an
> existing step like print_os_info or print_cpu_info.
>

Hi  David ,  print_cpu_info  does not sound like a great fit . Some info  like

LPAR Number: 14
LPAR Characteristics: Shared
LPAR Name: VM12

 Does not really belong there .

print_os_info   looks   better ,  it already contains  "container_info"  on Linux, so  I think this might fit .

Best regards, Matthias

> -----Original Message-----
> From: David Holmes <david.holmes at oracle.com<mailto:david.holmes at oracle.com>>
> Sent: Dienstag, 29. Januar 2019 05:17
> To: Baesken, Matthias <matthias.baesken at sap.com<mailto:matthias.baesken at sap.com>>; 'hotspot-
> dev at openjdk.java.net<mailto:dev at openjdk.java.net>' <hotspot-dev at openjdk.java.net<mailto:hotspot-dev at openjdk.java.net>>
> Subject: Re: RFR : 8217786: Provide virtualization related info in the hs_error
> file on linux s390x
>
> On 28/01/2019 10:23 pm, Baesken, Matthias wrote:
> >>
> >> Can't you include this information in an existing section of the error
> >> processing code instead of adding a new function that is empty
> >> everywhere except Linux?
> >>
> >
> > Hi David ,   do you mean  something like
> >
> >
> > #if defined(S390)
> >
> >    STEP("printing virtualization info")
> >   ...
> >
> > #endif
>
> No I was thinking more about just adding the virtualization info to an
> existing step like print_os_info or print_cpu_info.
>
> Cheers,
> David
> -----
>
> >   in   vmError.cpp  ?
> >
> > I thought about doing this.
> >
> >
> >   But  on the other hand ,  the now   still  empty
> os::pd_print_virtualization_info    in    platforms != linux
> >   might fill over time   ( we could  add  [at least for some platforms]   other
> virtualization  related  info ).
> >
> >
> > Best regards, Matthias
> >
> >
> >> -----Original Message-----
> >> From: David Holmes <david.holmes at oracle.com<mailto:david.holmes at oracle.com>>
> >> Sent: Montag, 28. Januar 2019 12:35
> >> To: Baesken, Matthias <matthias.baesken at sap.com<mailto:matthias.baesken at sap.com>>; 'hotspot-
> >> dev at openjdk.java.net<mailto:dev at openjdk.java.net>' <hotspot-dev at openjdk.java.net<mailto:hotspot-dev at openjdk.java.net>>
> >> Subject: Re: RFR : 8217786: Provide virtualization related info in the
> hs_error
> >> file on linux s390x
> >>
> >> Hi Matthias,
> >>
> >> On 28/01/2019 6:48 pm, Baesken, Matthias wrote:
> >>> Hello, please review  this change ; it adds  virtualization related info in
> the
> >> hs_error file on linux s390x .
> >>
> >> Can't you include this information in an existing section of the error
> >> processing code instead of adding a new function that is empty
> >> everywhere except Linux?
> >>
> >> Thanks,
> >> David
> >>
> >>> On linux s390x, we usually  (always?)   run in virtualized environments
> >> (LPAR and/or z/VM / KVM ).
> >>>
> >>> It is helpful for instance in support cases to get some information about
> the
> >> virtualized environment in the hs_error file .
> >>> A lot of info can be taken from the /proc/sysinfo file on linux s390x .
> >>>
> >>>
> >>> Bug/webrev :
> >>>
> >>> https://bugs.openjdk.java.net/browse/JDK-8217786
> >>>
> >>>
> >>> http://cr.openjdk.java.net/~mbaesken/webrevs/8217786.1/
> >>>
> >>>
> >>>
> >>> Best regards, Matthias
> >>>