criu: Add pidfd support #2259
criu: Add pidfd support #2259
Conversation
h0lyalg0rithm
force-pushed
the
pidfd_support
branch
5 times, most recently
from
406a571
to
9724703
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## criu-dev #2259 +/- ##
============================================
- Coverage 70.67% 70.50% -0.17%
============================================
Files 133 135 +2
Lines 33330 33379 +49
============================================
- Hits 23556 23534 -22
- Misses 9774 9845 +71
☔ View full report in Codecov by Sentry. |
h0lyalg0rithm
force-pushed
the
pidfd_support
branch
2 times, most recently
from
9d2321c
to
9b0a7ce
Compare
|
@h0lyalg0rithm Is it ok, if I write the zdtm test for this? |
|
@warusadura There is one bug that is to be fixed.Currently the patch lets you dump pidfd for processes which are not part of the current process tree. I have the test prepared where I fork the test and have pidfd on the child process. I cannot think about any thing which doesnt sound hacky. What do you think? |
Signed-off-by: Suraj Shirvankar <[email protected]>
h0lyalg0rithm
force-pushed
the
pidfd_support
branch
2 times, most recently
from
0d46cc3
to
4939627
Compare
sorry @h0lyalg0rithm I'm not sure :) |
| PidfdEntry *pidfde = info->pidfde; | ||
|
|
||
| pr_info("Creating new pidfd %" PRId64 "\n", pidfde->pid); | ||
| fd = pidfd_open(pidfde->pid, 0); |
There was a problem hiding this comment.
a task linked to fd can be dead and pidfd_open will fail in this case.
|
|
||
| test_init(argc, argv); | ||
|
|
||
| pidfd = pidfd_open(1, 0); |
There was a problem hiding this comment.
The test has to create more than one pidfd linked to different tasks and then checks that they are linked to proper tasks after C/R.
There was a problem hiding this comment.
@avagin I tried to write a test that would create a fork of a process and the parent process would monitor the pid of the child process. The dumping and restore process would first restore the parent process and the child process would still not be restored, hence the restore would fail.
I was thinking of restoring the pidfd after all the child processes have been restored, does that make sense.
To do this I would have to dump the pidfd seperate from the regular fd dump process.
| fail(); | ||
| } | ||
|
|
||
| if (pollfd.revents & POLLIN) { |
criu/image.c
Outdated
|
|
||
| img = open_image(CR_FD_INVENTORY, O_RSTR); | ||
| if (!img) | ||
| return -1; | ||
|
|
||
| if (pb_read_one(img, &he, PB_INVENTORY) < 0) | ||
| loaded = pb_read_one(img, &he, PB_INVENTORY); | ||
| if (loaded < 0) |
criu/include/magic.h
Outdated
| @@ -100,6 +100,7 @@ | |||
| #define BPFMAP_FILE_MAGIC 0x57506142 /* Alapayevsk */ | |||
| #define BPFMAP_DATA_MAGIC 0x64324033 /* Arkhangelsk */ | |||
| #define APPARMOR_MAGIC 0x59423047 /* Nikolskoye */ | |||
| #define PIDFD_MAGIC 0x59423447 | |||
There was a problem hiding this comment.
we tend to use location coordinates of a city as a magic number. For example, Nikolskoye is a city with the following coordinates (from Wikipedia) 59°42'N 30°47'E -> 0x59423047.
Signed-off-by: Suraj Shirvankar <[email protected]>
h0lyalg0rithm
force-pushed
the
pidfd_support
branch
from
4939627
to
6256538
Compare
|
A friendly reminder that this PR had no activity for 30 days. |
|
closing in favour of #2449 |
No description provided.