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

pass real fd when restore and dump ext file #2502

Closed

Conversation

duguhaotian
Copy link
Contributor

@duguhaotian duguhaotian commented Oct 24, 2024

Changes:

  1. dump ext file: should pass real fd in dumped process;
  2. restore ext file: should pass target fd which will open;

Background:
During the restoration process of an NPU device, it is necessary to distinguish between file descriptors (fd) to perform different operations.

  1. Record operations targeting the device file descriptor (fd);
  2. Replay the corresponding operations based on the fd.

rst0git and others added 30 commits July 7, 2023 15:45
This patch removes code introduced for compatibility with
Python 2 in commits:

  bf80fee (lib: correctly handle stdin/stdout (Python 3))

  b82f222 (lib: fix crit-recode fix for Python 2)

Signed-off-by: Radostin Stoyanov <[email protected]>
This patch removes the code for Python 2 compatibility introduced
with commit e65c7b5 (zdtm: Replace imp module with importlib).

Signed-off-by: Radostin Stoyanov <[email protected]>
This makes the error to mount cgroup hierarchy a bit less noisy:

Error (criu/cgroup.c:623): cg: Unable to mount cgroup2 : Invalid argument'

Instead of

Error (criu/cgroup.c:623): cg: Unable to mount cgroup2 : Invalid argument'
Error (criu/cgroup.c:715): cg: failed walking /proc/self/fd/-1/zdtmtst for empty cgroups: No such file or directory'

Signed-off-by: Michał Mirosław <[email protected]>
Log the mount and file that were the cause of failing a dump.

Signed-off-by: Michał Mirosław <[email protected]>
…socket.

Make debugging dump failures resulting in "sk unix: Can't dump half
of stream unix connection" errors easier.

Signed-off-by: Michał Mirosław <[email protected]>
Errors in early restore.log for status=1 from a subprocess are confusing,
esp. that they don't show what command failed. Since the result is
either ignored or logged anyway, mark the calls as "can fail".

Signed-off-by: Michał Mirosław <[email protected]>
Make logs about inaccessible mounts warnings, as the failures are
normally harmless (e.g. failure to read /dev/cgroup) and don't
make the CRIU run fail. (If it happens that the fsnotify can't
find a file, then to debug, full CRIU logs will be necessary anyway.)

Signed-off-by: Michał Mirosław <[email protected]>
These errors originate from the filesystem scanning in irmap.c and are mostly
benign. Nevertheless, if they do result in a failed irmap lookup, that failed
lookup is more interesting from an application perspective.

Signed-off-by: Michał Mirosław <[email protected]>
Fixes: checkpoint-restore#2222
Fixes: f1c8d38 ("kerndat: check if setsockopt IPV6_FREEBIND is supported")
Signed-off-by: Yan Evzman <[email protected]>
Signed-off-by: Andrei Vagin <[email protected]>
NR_fstat is a deprecated syscall, some
modern architectures such as riscv and
loongarch64 no longer support this syscall.
It is usually replaced by NR_statx.

NR_statx is supported since linux 4.10.

Signed-off-by: znley <[email protected]>
kerndat_has_ipv6_freebind creates a socket but doesn't close it.

Signed-off-by: Andrei Vagin <[email protected]>
Add generic wrappers for fchown() and fchmod() that skip the calls if
no changes are needed. This will allow to unify places where we can
avoid errors when no-op requests are not permitted.

Signed-off-by: Michał Mirosław <[email protected]>
Note: This removes the difference in calling convention of
restore_file_perms() returning -errno that was the only call that did
this in the caller.

From: Radosław Burny <[email protected]>
Signed-off-by: Michał Mirosław <[email protected]>
When CRIU is run with the task's credentials on restore, don't set uids
and gids. This avoids the need to modify the SECURE_NO_SETUID_FIXUP flag
which requires CAP_SETPCAP.

From: Andy Tucker <[email protected]>
Signed-off-by: Michał Mirosław <[email protected]>
Skip calling setgroups() when the list of auxiliary groups already has
the values we want.  This allows restoring into an unprivileged user
namespace where setgroups() is disabled.

From: Ambrose Feinstein <[email protected]>
Signed-off-by: Michał Mirosław <[email protected]>
…els.

When restoring on a kernel that has different number of supported
capabilities than checkpoint one, check that the extra caps are unset.

There are two directions to consider:

1) dump.cap_last_cap > restore.cap_last_cap
	- restoring might reduce the processes' capabilities if restored
	  kernel doesn't support checkpointed caps. Warn.

2) dump.cap_last_cap < restore.cap_last_cap
	- restoring will fill the extra caps with zeroes. No changes.

Note: `last_cap` might change without affecting `n_words`.

Signed-off-by: Michał Mirosław <[email protected]>
adrianreber and others added 26 commits August 16, 2024 09:24
When attempting to checkpoint a container with CUDA processes,
CRIU could fail with the following error:

	Error (criu/cr-dump.c:1791): Timeout reached. Try to interrupt: 1
	Error (cuda_plugin.c:143): cuda_plugin: Unable to read output of cuda-checkpoint: Interrupted system call
	Error (cuda_plugin.c:384): cuda_plugin: PAUSE_DEVICES failed with

In this situation, the target process is locked, but CRIU fails due to
a timeout and exits with an error. We need to make sure that the target
PID is unlocked in such case.

Signed-off-by: Radostin Stoyanov <[email protected]>
The `uninstall_module.py` script is a wrapper for the `pip uninstall`
command that enables support for specifying installation prefix
(i.e., `--prefix`). When this functionality is used, we intentionally
set `sys.path` to include only search paths for the specified prefix
to avoid unintentional uninstallation of packages in system paths.

Since `importlib_metadata` version 8.1.0, the `Distribution.from_name()`
method has been modified [1] to perform additional pre-processing of
Distribution objects [2] that requires loading distribution metadata
and results in the following error:

  File "/usr/local/lib/python3.12/site-packages/importlib_metadata/__init__.py", line 422, in <lambda>
    buckets = bucket(dists, lambda dist: bool(dist.metadata))
                                              ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/importlib_metadata/__init__.py", line 454, in metadata
    from . import _adapters
  File "/usr/local/lib/python3.12/site-packages/importlib_metadata/_adapters.py", line 3, in <module>
    import email.message
  File "/usr/lib64/python3.12/email/message.py", line 11, in <module>
    import quopri
  ModuleNotFoundError: No module named 'quopri'

This error occurs because we have excluded system paths from the list
of search paths (`sys.path`).

However, this pre-processing is not required for our use case, as we
only use the discovery mechanism of importlib_metadata to resolve the
metadata directory path of the module being uninstalled.

To fix this problem, this patch updates `uninstall_module` to avoid the
`from_name()` method and use `discover(name=package_name)` directly.

[1] python/importlib_metadata@a65c29a
[2] https://github.com/python/importlib_metadata/blob/a65c29ad/importlib_metadata/__init__.py#L391

Fixes: checkpoint-restore#2468

Signed-off-by: Radostin Stoyanov <[email protected]>
This patch fixes the following typos reported by codespell:

./test/others/bers/bers.c:394: dependin ==> depending, depend in
./criu/kerndat.c:837: hitted ==> hit

Signed-off-by: Radostin Stoyanov <[email protected]>
Some plugins (e.g., CUDA) may not function correctly when processes are
frozen using cgroups. This change introduces a mechanism to disable the
use of freeze cgroups during process seizing, even if explicitly
requested via the --freeze-cgroup option.

The CUDA plugin is updated to utilize this new mechanism to ensure
compatibility.

Signed-off-by: Andrei Vagin <[email protected]>
Adds a new "fault" to call dont_use_freeze_cgroup.
The presence of /dev/nvidiactl indicates that the system has a
compatible NVIDIA GPU driver installed and that the GPU is accessible to
the operating system.

Signed-off-by: Andrei Vagin <[email protected]>
This struct was being used un-initialized, meaning it
was filled with random garbage.

Mea culpa.

Signed-off-by: David Francis <[email protected]>
The topology parsing assumed that all parameter names were
30 characters or fewer, but

recommended_sdma_engine_id_mask

is 31 characters.

Make the maximum length a macro, and set it to 64.

Signed-off-by: David Francis <[email protected]>
It should help to investigate errors of fsconfig, fsmount and etc.

Signed-off-by: Andrei Vagin <[email protected]>
This patch fixes the following warnings that appear
when building an RPM package:

+ /usr/lib/rpm/redhat/brp-mangle-shebangs
*** WARNING: ./usr/src/debug/criu-4.0-1.fc42.x86_64/plugins/amdgpu/amdgpu_plugin_util.c is executable but has no shebang, removing executable bit
*** WARNING: ./usr/src/debug/criu-4.0-1.fc42.x86_64/plugins/amdgpu/amdgpu_plugin_util.h is executable but has no shebang, removing executable bit

Signed-off-by: Radostin Stoyanov <[email protected]>
By default, CRIU uses the path "/usr/lib/criu" to install and load
plugins at runtime. This path is defined by the `PLUGINDIR` variable
in Makefile.install and `CR_PLUGIN_DEFAULT` in `criu/include/plugin.h`.
However, some distribution packages might install the CRIU plugins at
"/usr/lib64/criu" instead. This patch updates the makefile to align
the path defined by `CR_PLUGIN_DEFAULT` with the value of `PLUGINDIR`.

Signed-off-by: Radostin Stoyanov <[email protected]>
We only use the last pid from the list in NSpid entry (from
/proc/<pid>/fdinfo/<pidfd>) while restoring pidfds.
The last pid refers to the pid of the process in the most deeply nested
pid namespace. Since CRIU does not currently support nested pid
namespaces, this entry is the one we want.

After Linux 6.9, inode numbers can be used to compare pidfds. pidfds
referring to the same process will have the same inode numbers. We use
inode numbers to restore pidfds that point to dead processes.

Signed-off-by: Bhavik Sachdev <[email protected]>
Process file descriptors (pidfds) were introduced to provide a stable
handle on a process. They solve the problem of pid recycling.

For a detailed explanation, see https://lwn.net/Articles/801319/ and
http://www.corsix.org/content/what-is-a-pidfd

Before Linux 6.9, anonymous inodes were used for the implementation of
pidfds. So, we detect them in a fashion similiar to other fd types that
use anonymous inodes by calling `readlink()`.
After 6.9, pidfs (a file system for pidfds) was introduced.
In 6.9 `S_ISREG()` returned true for pidfds, but this again changed with
6.10.
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/pidfs.c?h=v6.11-rc2#n285)
After this change, pidfs inodes have no file type in st_mode in
userspace.
We use `PID_FS_MAGIC` to detect pidfds for kernel >= 6.9
Hence, check for pidfds occurs before the check for regular files.

For pidfds that refer to dead processes, we lose the pid of the process
as the Pid and NSpid fields in /proc/<pid>/fdinfo/<pidfd> change to -1.
So, we create a temporary process for each unique inode and open pidfds
that refer to this process. After all pidfds have been opened we kill
this temporary process.

This commit does not include support for pidfds that point to a specific
thread, i.e pidfds opened with `PIDFD_THREAD` flag.

Fixes: checkpoint-restore#2258

Signed-off-by: Bhavik Sachdev <[email protected]>
Ensures that entries in /proc/<pid>/fdinfo/<pidfd> are same.

Signed-off-by: Bhavik Sachdev <[email protected]>
Ensure `pidfd_send_signal()` syscall works as expected after C/R.

Signed-off-by: Bhavik Sachdev <[email protected]>
Validate that pidfds can been used to send signals to different
processes after C/R using the `pidfd_send_signal()` syscall.

Signed-off-by: Bhavik Sachdev <[email protected]>
After, C/R of pidfds that point to dead processes their inodes might
change. But if two pidfds point to same dead process they should
continue to do so after C/R.

This test ensures that this happens by calling `statx()` on pidfds after
C/R and then comparing their inode numbers.

Support for comparing pidfds by using `statx()` and inode numbers was
introduced alongside pidfs. So if `f_type` of pidfd is not equal to
`PID_FS_MAGIC` then we skip this test.

signed-off-by: Bhavik Sachdev <[email protected]>
We get the read end of a pipe using `pidfd_getfd` and check if we can
read from it after C/R.

signed-off-by: Bhavik Sachdev <[email protected]>
We open a pidfd to a thread using `PIDFD_THREAD` flag and after C/R
ensure that we can send signals using it with `PIDFD_SIGNAL_THREAD`.

signed-off-by: Bhavik Sachdev <[email protected]>
The command `ruff <path>` has been deprecated and removed:
https://astral.sh/blog/ruff-v0.5.0#removed-deprecated-features

Signed-off-by: Radostin Stoyanov <[email protected]>
This patch fixes the following errors reported by ruff:

lib/pycriu/images/pb2dict.py:307:24: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
    |
305 |     elif field.type in _basic_cast:
306 |         cast = _basic_cast[field.type]
307 |         if pretty and (cast == int):
    |                        ^^^^^^^^^^^ E721
308 |             if is_hex:
309 |                 # Fields that have (criu).hex = true option set
    |

lib/pycriu/images/pb2dict.py:379:13: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
    |
377 |     elif field.type in _basic_cast:
378 |         cast = _basic_cast[field.type]
379 |         if (cast == int) and is_string(value):
    |             ^^^^^^^^^^^ E721
380 |             if _marked_as_dev(field):
381 |                 return encode_dev(field, value)
    |

Signed-off-by: Radostin Stoyanov <[email protected]>
This patch extends the inventory image with a `plugins` field that
contains an array of plugins which were used during checkpoint,
for example, to save GPU state. In particular, the CUDA and AMDGPU
plugins are added to this field only when the checkpoint contains
GPU state. This allows to disable unnecessary plugins during restore,
show appropriate error messages if required CRIU plugin are missing,
and migrate a process that does not use GPU from a GPU-enabled system
to CPU-only environment.

We use the `optional plugins_entry` for backwards compatibility. This
entry allows us to distinguish between *unset* and *missing* field:

- When the field is missing, it indicates that the checkpoint was
  created with a previous version of CRIU, and all plugins should be
  *enabled* during restore.

- When the field is empty, it indicates that no plugins were used during
  checkpointing. Thus, all plugins can be *disabled* during restore.

Signed-off-by: Radostin Stoyanov <[email protected]>
This patch adds two test plugins to verify that CRIU plugins listed
in the inventory image are enabled, while those that are not listed
can be disabled.

Signed-off-by: Radostin Stoyanov <[email protected]>
This patch blocks SIGCHLD during temporary process creation to prevent a
race condition between kill() and waitpid() where sigchld_handler()
causes `criu restore` to fail with an error.

Fixes: checkpoint-restore#2490

Signed-off-by: Bhavik Sachdev <[email protected]>
Signed-off-by: Radostin Stoyanov <[email protected]>
@avagin
Copy link
Member

avagin commented Oct 24, 2024

@duguhaotian
Copy link
Contributor Author

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.