Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CRIU removes libj9criu29 #18745

Merged
merged 1 commit into from
Jan 19, 2024
Merged

CRIU removes libj9criu29 #18745

merged 1 commit into from
Jan 19, 2024

Conversation

JasonFengJ9
Copy link
Member

@JasonFengJ9 JasonFengJ9 commented Jan 11, 2024 •
edited
Loading

@JasonFengJ9 JasonFengJ9 added comp:vm criu Used to track CRIU snapshot related work labels Jan 11, 2024
This was referenced Jan 11, 2024
Merged
Merged
Merged
Merged
Merged
Merged
@pshipton
Copy link
Member

I'm setting this as draft just because I want to get the latest OpenJDK changes into 0.43 before we diverge the extensions.

@pshipton pshipton marked this pull request as draft January 11, 2024 20:58
@JasonFengJ9
CRIU removes libj9criu29
de00895 
This library is not needed after all its functionalities are moved into
java.base;
setupJNIFieldIDsAndCRIUAPI() is still required to check CRIU library
loading before performing a checkpointJVM().

Signed-off-by: Jason Feng <fengj@ca.ibm.com>
@JasonFengJ9
Copy link
Member Author

The latest JDK21 jdk-21.0.2+13 hasn't been merged into 0.43 release yet.

@JasonFengJ9 JasonFengJ9 mentioned this pull request Jan 17, 2024
Merged
@JasonFengJ9 JasonFengJ9 marked this pull request as ready for review January 17, 2024 21:31
@JasonFengJ9
Copy link
Member Author

ibmruntimes/openj9-openjdk-jdk21#123 is merged.
Moving this out of draft state.

tajila
tajila approved these changes Jan 17, 2024
View reviewed changes
Copy link
Contributor

@tajila tajila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should get this into 0.43 if possible

@tajila
Copy link
Contributor

tajila commented Jan 17, 2024

jenkins test sanity alinux64 jdk21

@tajila
Copy link
Contributor

tajila commented Jan 17, 2024

jenkins test sanity zlinux jdk17

@pshipton
Copy link
Member

I think we should get this into 0.43 if possible

Is it worth a delay to the release?

@pshipton
Copy link
Member

Are there any security aspects to having the library available when it's not used?

@tajila
Copy link
Contributor

tajila commented Jan 17, 2024

Is it worth a delay to the release?

No

Are there any security aspects to having the library available when it's not used?

Im only concerned about the missing check for the native library load in CRIUSupport::isCRIRSupportEnabled

@JasonFengJ9
Copy link
Member Author

Build_JDK21_aarch64_linux_Personal — Build FAILED

17:37:53 curl: (56) OpenSSL SSL_read: Connection reset by peer, errno 104
Infra error

@pshipton
Copy link
Member

As I understand it there is a library containing the following natives but no equivalent Java code, so any app could add the java code and call these natives.

Java_org_eclipse_openj9_criu_CRIUSupport_checkpointJVMImpl
Java_org_eclipse_openj9_criu_CRIUSupport_getRestoreSystemProperites
Java_org_eclipse_openj9_criu_CRIUSupport_setupJNIFieldIDsAndCRIUAPI

@pshipton
Copy link
Member

No RC1 builds have been started, and I need to update an OpenJ9 change to cover jdk21 so there is some time to get this in.

@tajila
Copy link
Contributor

tajila commented Jan 17, 2024

jenkins test sanity alinux64 jdk21

@JasonFengJ9
Copy link
Member Author

19:17:27 gmake[3]: *** No rule to make target '/home/jenkins/workspace/Build_JDK17_s390x_linux_Personal/build/linux-s390x-server-release/vm/runtime/libj9criu29.so', needed by '/home/jenkins/workspace/Build_JDK17_s390x_linux_Personal/build/linux-s390x-server-release/support/modules_libs/openj9.criu/default/libj9criu29.so'. Stop.

This PR depends on

@pshipton
Copy link
Member

jenkins test sanity alinux64 jdk21 depends ibmruntimes/openj9-openjdk-jdk21#114

@pshipton
Copy link
Member

jenkins test sanity zlinux jdk17 depends ibmruntimes/openj9-openjdk-jdk17#306

@JasonFengJ9
Copy link
Member Author

Im only concerned about the missing check for the native library load in CRIUSupport::isCRIRSupportEnabled

Just realized that 0.43 doesn't have

That PR was to prepare the adoption of jdk.crac

Since the issue wasn't in 0.43, this PR isn't needed there either.

@tajila
Copy link
Contributor

tajila commented Jan 18, 2024

Since the issue wasn't in 0.43, this PR isn't needed there either.

okay, thats good

@tajila tajila merged commit 31c6afe into eclipse-openj9:master Jan 19, 2024
7 checks passed
@JasonFengJ9 JasonFengJ9 deleted the rmj9criu branch January 19, 2024 15:40
@ThanHenderson ThanHenderson mentioned this pull request Feb 5, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tajila tajila tajila approved these changes

Assignees
No one assigned
Labels
comp:vm criu Used to track CRIU snapshot related work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JasonFengJ9 @pshipton @tajila