-
Notifications
You must be signed in to change notification settings - Fork 319
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
criu: do not set network_lock if not specified #1628
base: main
Are you sure you want to change the base?
Conversation
commit c4f8c87 introduced the issue. Closes: containers#1627 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@lsm5 the failures depend on some broken dependencies:
|
fix the function signature for criu_set_network_lock, and check errors from libcriu. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
cbdc622
to
588eff1
Compare
@@ -907,7 +934,7 @@ libcrun_container_restore_linux_criu (libcrun_container_status_t *status, libcru | |||
ret = libcriu_wrapper->criu_set_root (root); | |||
if (UNLIKELY (ret != 0)) | |||
{ | |||
ret = crun_make_error (err, 0, "error setting CRIU root to `%s`", root); | |||
ret = crun_make_error (err, -errno, "error setting CRIU root to `%s`", root); |
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.
Recursive grep in crun sources
$ grep -r crun_make_error . | grep ", errno," | wc -l
477
$ grep -r crun_make_error . | grep ", -errno," | wc -l
1
$
There is only one occurence of -errno
. I would have expected 0
, -ret
or errno
.
Re: cs10, I'll do something about the catatonit dependency. Re: rhel9, we've removed rhel jobs from our other projects because of older packages causing frequent failures. CentOS Stream works mostly well upstream. Re: cs9, llvm 19.1.5 has been built already https://kojihub.stream.centos.org/koji/packageinfo?packageID=1160 so I guess just a matter of time before it lands in the repos. Your call if you wanna block or proceed with this PR. I can check with the maintainers about ETA, but I doubt they'd know the exact dates. |
|
||
ret = libcriu_wrapper->criu_dump (); | ||
if (UNLIKELY (ret != 0)) | ||
return crun_make_error (err, 0, | ||
return crun_make_error (err, -ret, |
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.
I tried to look in the criu source code to see what kind of values criu_dump()
could return.
I followed the functions:
int criu_dump(void)
which calls
int criu_local_dump(criu_opts *opts)
which calls
static int dump(bool pre_dump, criu_opts *opts)
Maybe the return value could be 1
if it comes from
https://github.com/checkpoint-restore/criu/blob/32d5a766eeb8a41e52a1c0234bbf2c5757f64b4b/lib/c/criu.c#L1580
?
Using -1
as second argument of crun_make_error()
causes that function to return 0
which would be surprising.
Line 61 in a48818a
return -status - 1; |
Sorry, this comment is a bit vague. I haven't found any reproducer for the mentioned scenario to happen.
commit c4f8c87 introduced the issue.
also improve error handling from criu APIs
Closes: #1627