-
Notifications
You must be signed in to change notification settings - Fork 729
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
Do unprivileged CRIU dump if requested+supported #14836
Conversation
The corresponding Open Liberty PR is OpenLiberty/open-liberty#20683 |
e63c98b
to
7d81d4d
Compare
7d81d4d
to
e90bad6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just some formatting things
runtime/criusupport/criusupport.cpp
Outdated
systemReturnCode = J9_CRIU_UNPRIVILEGED_NO_ERROR; | ||
criu_set_unprivileged_ptr(TRUE); | ||
} | ||
else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
format: } else {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, thanks.
runtime/criusupport/criusupport.cpp
Outdated
@@ -256,6 +264,8 @@ Java_org_eclipse_openj9_criu_CRIUSupport_checkpointJVMImpl(JNIEnv *env, | |||
const char *nlsMsgFormat = NULL; | |||
UDATA msgCharLength = 0; | |||
IDATA systemReturnCode = 0; | |||
fn_criu_set_unprivileged criu_set_unprivileged_ptr = NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use camelCase for naming here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, thanks.
runtime/criusupport/criusupport.cpp
Outdated
@@ -350,6 +360,30 @@ Java_org_eclipse_openj9_criu_CRIUSupport_checkpointJVMImpl(JNIEnv *env, | |||
goto closeWorkDirFD; | |||
} | |||
|
|||
if (TRUE == unprivileged) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be JNI_TRUE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, thanks.
runtime/criusupport/criusupport.cpp
Outdated
@@ -36,6 +37,8 @@ | |||
|
|||
extern "C" { | |||
|
|||
typedef void (*fn_criu_set_unprivileged)(bool unprivileged); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it defined with bool
in the criu API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I actually added the function since it is missing from checkpoint-restore/criu#1155 however other functions taking a boolean (e.g. criu_set_shell_job
) also define their booleans as bool
.
e90bad6
to
1577bd4
Compare
runtime/criusupport/criusupport.cpp
Outdated
} | ||
if ((NULL == dlerrorReturnString) && (NULL != criuSetUnprivileged)) { | ||
systemReturnCode = J9_CRIU_UNPRIVILEGED_NO_ERROR; | ||
criuSetUnprivileged(TRUE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please JNI_FALSE != unprivileged
for consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, thanks.
If the user requests an unprivileged CRIU dump we check if a non-NULL `criu_set_unprivileged` symbol can be found via dlsym() and if so call it with `true` as the parameter. If the user requests an unprivileged CRIU dump but the symbol cannot be found we throw an exception. If the user does not request an unprivileged CRIU dump we do nothing as CRIU (currently) defaults to privileged. Also link against libdl to get access to dlsym(). Signed-off-by: Younes Manton <[email protected]>
1577bd4
to
cbb83ab
Compare
Jenkins compile xlinuxcriu jdk11 |
Jenkins test sanity win jdk8 |
Jenkins test sanity plinux jdk11 |
If the user requests an unprivileged CRIU dump we check
if a non-NULL
criu_set_unprivileged
symbol can be found viadlsym() and if so call it with
true
as the parameter.If the user requests an unprivileged CRIU dump but the symbol
cannot be found we throw an exception.
If the user does not request an unprivileged CRIU dump we do
nothing as CRIU (currently) defaults to privileged.
Also link against libdl to get access to dlsym().
Signed-off-by: Younes Manton [email protected]