RFR: 8261242: [Linux] OSContainer::is_containerized() returns true when run outside a container [v2]

[Thomas Stuefe]

On Sat, 13 Apr 2024 18:29:59 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains ten additional commits since the last revision:
>> 
>>  - Merge branch 'master' into jdk-8261242-is-containerized-fix
>>  - jcheck fixes
>>  - Fix tests
>>  - Implement Metrics.isContainerized()
>>  - Some clean-up
>>  - Drop cgroups testing on plain Linux
>>  - Implement fall-back logic for non-ro controller mounts
>>  - Make find_ro static and local to compilation unit
>>  - 8261242: [Linux] OSContainer::is_containerized() returns true
>
> src/hotspot/os/linux/cgroupSubsystem_linux.cpp line 170:
> 
>> 168:     }
>> 169:   }
>> 170:   return false;
> 
> An alternative, simpler, no need for modifying source string:
> 
> static bool find_ro_opt(const char* o) {
>   return strcmp(o, "ro") || strstr(o, ",ro") || strstr(o, "ro,");
> }

Please disregard my comment. Albeit longer, your version is clearer to read and more fault tolerant.

> src/hotspot/os/linux/cgroupSubsystem_linux.cpp line 351:
> 
>> 349:     //
>> 350:     // We collect the read only mount option in the cgroup infos so as to have that
>> 351:     // info ready when determining is_containerized().
> 
> Here, and in other places: a comment indicating the line format we scan would be appreciated, possibly with argument numbers. Saves the casual code reader from looking into proc man page. Even just pasting the example line for proc manpage would be fine (https://man7.org/linux/man-pages/man5/proc.5.html) (but with order adapted to your scanf call, they count major:minor as one)

Trying to parse the `%s%*[^-]-`

So, %s parses the mount options, until we encounter whitespace. Then %*[^-]- parses everything that is not a dash, until we encounter the dash? Then we eat the dash? This is to skip the optionals?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18201#discussion_r1567754861
PR Review Comment: https://git.openjdk.org/jdk/pull/18201#discussion_r1567767209