RFR : 8196062 : Enable docker container related tests for linux ppc64le

[Bob Vandette]

> On Jan 24, 2018, at 11:33 AM, Baesken, Matthias <matthias.baesken at sap.com> wrote:
> 
> Thanks  for  your input ;
> 
>> Can you add "return;" in each test for subsystem not found messages and
>> remove these 3 lines OR move your tests for NULL & messages inside.   ...
> 
> Sure I can do so.
> 
> Regarding the test / platform handling   improvement  you propose - I would prefer  doing this in a separate change  (or when later enabling  Docker detection + testing on our  other additional  platform  zLinux  / s390x
>  which supports Docker as well ).
> 
I’m not an official reviewer so we’ll have to see what others think.  Typically platform extensibility is
taken into account after there’s more than 1 platform requiring support.   Churning the sources
even if they are tests, make maintaining and back-porting features more difficult.

Bob.


> 
> Best Regards, Matthias
> 
> 
>> -----Original Message-----
>> From: Bob Vandette [mailto:bob.vandette at oracle.com]
>> Sent: Mittwoch, 24. Januar 2018 16:14
>> To: Baesken, Matthias <matthias.baesken at sap.com>
>> Cc: hotspot-dev at openjdk.java.net; Lindenmaier, Goetz
>> <goetz.lindenmaier at sap.com>; Langer, Christoph
>> <christoph.langer at sap.com>; Doerr, Martin <martin.doerr at sap.com>;
>> Mikhailo Seledtsov <mikhailo.seledtsov at oracle.com>
>> Subject: Re: RFR : 8196062 : Enable docker container related tests for linux
>> ppc64le
>> 
>> osContainer_linux.cpp:
>> 
>> Can you add "return;" in each test for subsystem not found messages and
>> remove these 3 lines OR move your tests for NULL & messages inside.  The
>> compiler can
>> probably optimize this but I’d prefer more compact code.
>> 
>> if (memory == NULL || cpuset == NULL || cpu == NULL || cpuacct == NULL) {
>> 
>> 342     return;
>> 343   }
>> 
>> 
>> The other changes in osContainer_linux.cpp look ok.
>> 
>> I forwarded your test changes to Misha, who wrote these.
>> 
>> Since it’s likely that other platforms, such as aarch64, are going to run into the
>> same problem,
>> It would have been better to enable the tests based on the existence of an
>> arch specific
>> Dockerfile-BasicTest-{os.arch} rather than enabling specific arch’s in
>> VPProps.java.
>> This approach would reduce the number of changes significantly and allow
>> support to
>> be added with 1 new file.
>> 
>> You wouldn’t need "String dockerFileName =
>> Common.getDockerFileName();”
>> in every test.  Just make DockerTestUtils automatically add arch.
>> 
>> Bob.
>> 
>> 
>> 
>>> On Jan 24, 2018, at 9:24 AM, Baesken, Matthias
>> <matthias.baesken at sap.com> wrote:
>>> 
>>> Hello,  could you please review the following change :  8196062 :  Enable
>> docker container related tests for linux ppc64le  .
>>> 
>>> It  adds  docker container testing   for linux ppc64 le  (little endian) .
>>> 
>>> A number of things had to be done :
>>> 	• Add a  separate  docker file
>> test/hotspot/jtreg/runtime/containers/docker/Dockerfile-BasicTest-ppc64le
>> for linux ppc64 le     which uses   Ubuntu ( the  Oracle Linux 7.2  used  for
>> x86_64  seems not to be available for ppc64le )
>>> 	• Fix  parsing    /proc/self/mountinfo    and   /proc/self/cgroup  in
>> src/hotspot/os/linux/osContainer_linux.cpp    , it could  not  handle  the
>> format seen  on SUSE LINUX 12.1 ppc64le (Host)  and  Ubuntu (Docker
>> container)
>>> 	• Add a bit  more logging
>>> 
>>> 
>>> Webrev :
>>> 
>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8196062/
>>> 
>>> 
>>> Bug :
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8196062
>>> 
>>> 
>>> After these adjustments I could run the    runtime/containers/docker    -
>> jtreg tests successfully .
>>> 
>>> 
>>> Best regards, Matthias
>