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

[Thomas Stüfe]

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>
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>
> *Sent:* Dienstag, 29. Januar 2019 09:23
> *To:* Baesken, Matthias <matthias.baesken at sap.com>; David Holmes <
> david.holmes at oracle.com>
> *Cc:* 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> 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>
> > Sent: Dienstag, 29. Januar 2019 05:17
> > To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
> > dev at openjdk.java.net' <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>
> > >> Sent: Montag, 28. Januar 2019 12:35
> > >> To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
> > >> dev at openjdk.java.net' <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
> > >>>
>
>