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 error logging to kerndat init #1125

Merged
merged 1 commit into from
Jul 9, 2020

Conversation

angieenee
Copy link

CRIU sometimes returns 1 from main() with no explanation.
These changes add additional logging for initialization errors in kerndat.

There may be discourse surrounding the messages themselves -- specific alternatives are written and recorded elsewhere for each "foo failed" message. Given the large number of functions I've written error messages for in this commit, it may be safer to start with generic "foo failed" messages. These can be replaced depending on the preference of the CRIU maintainers.

Signed-off-by: Angie Ni [email protected]

@angieenee
Copy link
Author

angieenee commented Jun 29, 2020

I'm currently looking into the travis failures.
Seems I opened this PR a little prematurely. Apologies for the inconvenience!

@0x7f454c46
Copy link
Member

Please, check Travis-CI, it's very red.
Seems like criu check fails after this.

@avagin
Copy link
Member

avagin commented Jul 8, 2020

Fixed bug + log levels from previous commit (#10) …

The commit history of your pull request has to be clean. If you find a bug in one of your commits, you don't need to create another commit to fix it, instead of this, you have to merge the fix into the target commit.

criu/kerndat.c Outdated
ret = kerndat_has_thp_disable();
if (check_pagemap()) {
pr_err("check_pagemap failed when initializing kerndat.\n");
ret = 1;
Copy link
Member

Choose a reason for hiding this comment

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

ret = -1

nit: usually, we use -1 or another negative value in an error case

Copy link
Author

Choose a reason for hiding this comment

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

Ah, thanks for letting me know! That's a good one to know :)

CRIU sometimes returns 1 from main() with no explanation.
These changes add additional logging for initialization errors in kerndat.

Signed-off-by: Angie Ni <[email protected]>
@avagin avagin merged commit 483792d into checkpoint-restore:criu-dev Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants