On Sat, 13 Apr 2024 18:29:59 GMT, Thomas Stuefe <stu...@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