On Fri, 9 Jan 2026 19:42:11 GMT, Chris Plummer <[email protected]> wrote:
>> Eunbin Son has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental views will show differences >> compared to the previous content of the PR. The pull request contains one >> new commit since the last revision: >> >> JDK-8374341: Remove unverified systemd-coredump handling code > > BTW, we currently have an issue with our own internal testing when dealing > with core files. Many hosts end up causing this "This system uses a crash > report tool.." message, so we end up skipping the core file tests. The cause > is the crash report tool being used to gzip the core file. I'm thinking of > adding support in CoreUtils to check for core.<pid>.gz, and unzip in that > case. I think the code to do this would go exactly where this PR is removing > the coredumpctl support. > > And regarding the removal of the coredumpctl support, this is the review of > the change that introduced it: > > https://mail.openjdk.org/pipermail/serviceability-dev/2021-January/035127.html > > So this clearly worked at one point. However, in the original webrev (before > feedback and changes), the code was in CiReplayBase.java and did not do the > `endsWith("systemd-coredump")` check. So perhaps it worked then, but then > when it was moved to CoreUtils.java during the review process, it was broken > and not noticed. > > I'm not so sure removing the coredumpctl support at this point is the right > thing to do. How was the \s error detected, and why do we think the right > thing to do at this point is to remove this support rather than fix it? Hi @plummercj I am presuming the the single backslash s typo was found by manual inspection or with a tool, @thswlsqls may want to confirm. It looks wrong. This match can never have worked? My system defaults to: # cat /proc/sys/kernel/core_pattern |/usr/lib/systemd/systemd-coredump %P %u %g %s %t %c %h %d %F ..so the match doesn't work, would never use the code that would be enabled here. BUT I think it would have worked by chance if the pattern was just "|/usr/lib/systemd/systemd-coredump" with no splitting possible or needed, and that may have been what was tested when integrated. I raised the point that correcting the typo needs testing to check it works as intended. i.e. we don't just appy the typo change. And an option is to remove the code if not tested or known good... But can coredumpctl really be useful here? If I change it to make the match work, and also make it print what the attempt to run "coredumpctl" sees, I get: Output and diagnostic info for process 1807382 was saved into 'pid-1807382-output.log' Hint: You are currently not seeing messages from other users and the system. Users in groups 'adm', 'systemd-journal', 'wheel' can see all messages. Pass -q to turn off this notice. No journal files were opened due to insufficient permissions. ..and then goes on to throw new SkippedException("This system uses a crash report tool"); (which it does if the match of systemd-coredump fails). So I see that recognising the pipe symbol is really benficial, as it prompts me to fix the system. But being so specific about matching systemd-coredump I'm not sure about the benefit. ------------- PR Comment: https://git.openjdk.org/jdk/pull/28984#issuecomment-3738241630
