-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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: return error when checking for min version #18857
criu: return error when checking for min version #18857
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@edsantiago PTAL |
I like the idea of handling errors, of course, but still hate the "CRIU is too old and I'm not going to tell you what version you have installed nyah nyah". Is there any way to rework the logic so the too-old message code does the comparison and can therefore display the current version? |
Sounds good, I think I can only return an error from this function then the caller just needs to do |
5279a3f
to
9e3d7dc
Compare
There is weird issue containers#18856 which causes the version check to fail. Return the underlying error in these cases so we can see it and debug it. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
9e3d7dc
to
ab502fc
Compare
Nice, thank you! LGTM. |
/lgtm |
There is weird issue #18856 which causes the version check to fail. Return the underlying error in these cases so we can see it and debug it.
Does this PR introduce a user-facing change?