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

Add CRIU support system property #14270

Merged
merged 1 commit into from
Jan 13, 2022
Merged

Conversation

tajila
Copy link
Contributor

@tajila tajila commented Jan 13, 2022

Add CRIU support system property

Related to #14016

Signed-off-by: Tobi Ajila [email protected]

@tajila
Copy link
Contributor Author

tajila commented Jan 13, 2022

@JasonFengJ9 @pshipton please review

@tajila tajila added comp:vm criu Used to track CRIU snapshot related work labels Jan 13, 2022
@llxia
Copy link
Contributor

llxia commented Jan 13, 2022

Thanks @tajila .
FYI @renfeiw please add TKG support accordingly.

/*[IF CRIU_SUPPORT]*/
initializedProperties.put("org.eclipse.openj9.criu.isCRIUCapable", "true"); //$NON-NLS-1$ //$NON-NLS-1$
/*[ELSE] CRIU_SUPPORT
initializedProperties.put("org.eclipse.openj9.criu.isCRIUCapable", "false"); //$NON-NLS-1$ //$NON-NLS-1$
Copy link
Member

Choose a reason for hiding this comment

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

I would hope any app/test assume org.eclipse.openj9.criu.isCRIUCapable is false if the system property is not set, then there is no need to set it false explicitly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its not set. Its just that it will return null. If @llxia is fine with that then I can remove the else case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess Lan will have to deal with the NULL case anyways, for JDKs that dont have this property at all so Ill just remove it.

@@ -618,6 +618,10 @@ private static void ensureProperties(boolean isInitialization) {
initializedProperties.put("java.specification.name", "Java Platform API Specification"); //$NON-NLS-1$ //$NON-NLS-2$
initializedProperties.put("com.ibm.oti.configuration", "scar"); //$NON-NLS-1$

/*[IF CRIU_SUPPORT]*/
initializedProperties.put("org.eclipse.openj9.criu.isCRIUCapable", "true"); //$NON-NLS-1$ //$NON-NLS-1$
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: //$NON-NLS-2$ is for second NLS comment.

Copy link
Member

@JasonFengJ9 JasonFengJ9 left a comment

Choose a reason for hiding this comment

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

LGTM

@pshipton pshipton merged commit 32e330c into eclipse-openj9:master Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:vm criu Used to track CRIU snapshot related work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants