Improve interaction between CRIU and CRaC configuration options

ibmruntimes / openj9-openjdk-jdk

Extensions for OpenJDK for Eclipse OpenJ9

GNU General Public License v2.0

17 stars 73 forks source link

Improve interaction between CRIU and CRaC configuration options #800

Closed keithc-ca closed 3 months ago

keithc-ca commented 3 months ago

CRaC support requires CRIU support. If CRIU support is disabled explicitly and CRaC is enabled explicitly, configuration will now fail rather than incur a compile error later.

Otherwise, if CRaC support is requested explictly, CRIU support will implicitly be enabled if that is not the default for CRIU.

Also, if CRIU support is explicitly disabled, CRaC will be implicitly disabled if that is not the default for CRaC.

JasonFengJ9 commented 3 months ago

CRaC always requires CRIU. Can we assume --enable-crac-support implies --enable-criu-support instead of checking CRIU and throwing an error otherwise?

--enable-criu-support enables CRIU
--enable-crac-support enabled CRIU and CRaC

@tajila thoughts?

keithc-ca commented 3 months ago

I suppose we could check the options the other way around, so explicitly saying --enable-crac-support would provide a default for CRIU, but I'm not sure that's much better or substantially different. My motivation for this change was due to compile errors that result if you just say --disable-criu-support on a platform where CRaC is enabled by default. This will prevent those compile errors by disabling CRaC if CRIU is disabled.

There are three valid combinations of the two configuration options; this does not change that, but it does point out when the fourth, invalid combination is requested.

JasonFengJ9 commented 3 months ago

How about the following?

--enable-criu-support enables CRIU
--enable-crac-support enabled CRaC where CRIU is enabled by default, otherwise an error will be thrown
--disable-criu-support disables CRIU and CRaC
--disable-crac-support disables CRaC

keithc-ca commented 3 months ago

How about the following?

That seems reasonable to me. @tajila Do you agree?

Using --enable-crac-support in combination with --disable-criu-support should still be disallowed.

JasonFengJ9 commented 3 months ago

Using --enable-crac-support in combination with --disable-criu-support should still be disallowed.

Right, I think a configuration error can tell the user this is an invalid combination.

tajila commented 3 months ago

That seems reasonable to me. @tajila Do you agree?

Using --enable-crac-support in combination with --disable-criu-support should still be disallowed.

Looks good to me.

keithc-ca commented 3 months ago

Updated as suggested.

JasonFengJ9 commented 3 months ago

jenkins compile amac,alinux64 jdknext

JasonFengJ9 commented 3 months ago

@pshipton pls review/merge

pshipton commented 3 months ago

jenkins compile amac jdknext