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.

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

Restore behavior on platforms where CRIU is supported but not in use #717

Merged
merged 1 commit into from
Jan 5, 2024

Conversation

WilburZjh
Copy link
Contributor

@WilburZjh WilburZjh commented Jan 4, 2024

This PR aims to fix an issue on how to read random data and
get random bytes on platforms where CRIU is supported, but
not in use by enabling it via a --enable-criu-support JVM
option.

Issue eclipse-openj9/openj9#18602

@pshipton
Copy link
Member

pshipton commented Jan 4, 2024

This is to fix eclipse-openj9/openj9#18602

Copy link
Member

@keithc-ca keithc-ca 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 the commit message and the summary here could be improved. Rather than describe how the problem was fixed, explain what problem is fixed.

@keithc-ca
Copy link
Member

We'll also want a copy of this (once finalized) for https://github.com/ibmruntimes/openj9-openjdk-jdk22.

@keithc-ca
Copy link
Member

The content looks good, but it doesn't appear that you've addressed my concern about the commit message or the summary here. From my perspective, the essence of this is to restore the behavior (actually read random data) on platforms where CRIU is supported, but not in use.

@WilburZjh
Copy link
Contributor Author

The content looks good, but it doesn't appear that you've addressed my concern about the commit message or the summary here. From my perspective, the essence of this is to restore the behavior (actually read random data) on platforms where CRIU is supported, but not in use.

Yes, I just updated the code. Now Im updating the commit message.

This PR aims to fix an issue on how to read random data and
get random bytes on platforms where CRIU is supported, but
not in use by enabling it via a --enable-criu-support JVM
option.
@WilburZjh WilburZjh changed the title Change to use ENDIF rather than ELSE for CRIU_SUPPORT Restore behavior on platforms where CRIU is supported but not in use Jan 4, 2024
@pshipton
Copy link
Member

pshipton commented Jan 4, 2024

I see the commit message was updated. Is this ready to be removed from draft state?

@WilburZjh
Copy link
Contributor Author

I see the commit message was updated. Is this ready to be removed from draft state?

I rerun the failed tests on the initial code without using the return, and the failed tests are passed. Now I am building the JDKs and rerun on the failed cases to ensure that the failed cases are passed.

Copy link
Member

@keithc-ca keithc-ca left a comment

Choose a reason for hiding this comment

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

Approved, assuming testing will be successful.

@pshipton
Copy link
Member

pshipton commented Jan 5, 2024

The change looked correct so I went ahead and merged it for jdk21 and we started a Semeru 0.42 M3 build, which is looking (very) good.

@WilburZjh
Copy link
Contributor Author

Build is passed, and the failed cases are passed. Change from draft to ready for review.

@jasonkatonica FYI

@WilburZjh WilburZjh marked this pull request as ready for review January 5, 2024 13:49
@pshipton pshipton merged commit fb79ca3 into ibmruntimes:openj9 Jan 5, 2024
2 checks passed
tajila pushed a commit to tajila/openj9-openjdk-jdk that referenced this pull request Mar 27, 2024
Update RestrictedSecurity flags, alter debug comments and profile name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants