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

Preliminary set of changes to support dumping BPF map files. A major task is yet to be completed: parsing BPF file info from the proc filesystem in the function parse_fdinfo_pid_s() #1036

Closed
wants to merge 291 commits into from

Conversation

criupatchwork
Copy link

This PR is autogenerated from: https://patchwork.criu.org/series/4130

rst0git and others added 30 commits January 24, 2020 10:46
In Py2 `range` returns a list and `xrange` creates a sequence object
that evaluates lazily. In Py3 `range` is equivalent to `xrange` in Py2.

Signed-off-by: Radostin Stoyanov <[email protected]>
Signed-off-by: Andrei Vagin <[email protected]>
Signed-off-by: Pavel Tikhomirov <[email protected]>
Signed-off-by: Andrei Vagin <[email protected]>
As discussed on the mailing list, current .py files formatting does not
conform to the world standard, so we should better reformat it. For this
the yapf tool is used. The command I used was

yapf -i $(find -name *.py)

Signed-off-by: Pavel Emelyanov <[email protected]>
Most of the typos were found by codespell.

Signed-off-by: Radostin Stoyanov <[email protected]>
Signed-off-by: Andrei Vagin <[email protected]>
This patch does not introduce any functional changes.

Signed-off-by: Radostin Stoyanov <[email protected]>
Signed-off-by: Andrei Vagin <[email protected]>
The function accept_proxy_to_cache() is a wrapper around accept().
Use accept() directly instead.

Signed-off-by: Radostin Stoyanov <[email protected]>
Signed-off-by: Andrei Vagin <[email protected]>
Signed-off-by: Radostin Stoyanov <[email protected]>
Signed-off-by: Andrei Vagin <[email protected]>
Reduce code duplication by introducing a strflags() macro which maps
a flag to corresponding string.

Signed-off-by: Radostin Stoyanov <[email protected]>
This patch makes various private variables and procedures static.

These changes conform to the following code style conventions:

 When declaring pointer data or a function that returns a pointer type,
 the preferred use of ``*`` is adjacent to the data name or function
 name and not adjacent to the type name.

 Statements longer than 80 columns will be broken into sensible chunks,
 unless exceeding 80 columns significantly increases readability and
 does not hide information. Descendants are always substantially
 shorter than the parent and are placed substantially to the right.
 The same applies to function headers with a long argument list.

from https://www.kernel.org/doc/Documentation/process/coding-style.rst

The function declarations {send,recv}_image() from img-remote.h are
removed because they do not have a corresponding implementation.

The following functions are made static because they are not used
outside img-remote.c:
* {send,recv}_image_async()
* {read,write}_remote_header()
* socket_set_non_blocking()

Signed-off-by: Radostin Stoyanov <[email protected]>
There is no need to print an error message, __xalloc() does that.

Signed-off-by: Radostin Stoyanov <[email protected]>
When an interrupt signal (even SIGWINCH when strace is used) is
received while epoll_wait() sleeps, it will return a value of
-1 and set errno to EINTR, which is not an error and should be
ignored.

Signed-off-by: Radostin Stoyanov <[email protected]>
There is no need to pass the values for address and port as arguments
when creating a TCP server. The external `opts` object, which provides
opts.addr and opts.port, is accessible in all components that require
these values.

With this change, a value specified with the `--address` option will
used by image-cache in the same way as with page-server.

Example:
criu image-cache --address 127.0.0.1 --port 1234
criu page-server --address 127.0.0.1 --port 1234

Signed-off-by: Radostin Stoyanov <[email protected]>
The variable name 'remote_sk' is shorter than 'proxy_to_cache_fd' and
it is more similar to 'page_server_sk' (used in criu/page-xfer.c).

Signed-off-by: Radostin Stoyanov <[email protected]>
The name 'local_sk' is shorter than 'local_req_fd', and it is more
similar to the name 'page_server_sk' used in criu/page-xfer.c

Signed-off-by: Radostin Stoyanov <[email protected]>
When saddr.ss_family is AF_INET6 we should cast &saddr to
(struct sockaddr_in6 *).

Signed-off-by: Radostin Stoyanov <[email protected]>
Combine the functionality of socket_set_non_blocking() and
socket_set_blocking() into a new function, and move it in
criu/util.c to enable reusability throughout the code base.

Signed-off-by: Radostin Stoyanov <[email protected]>
Combine the macro constants DUMP_FINISH and RESTORE_FINISH, into
a single one, called FINISH.

In addition, replace the key-word strings used by the above-mentioned
constants, and NULL_SNAPSHOT_ID, with a \0 character that will be used
to indicate a "finish" message.

Signed-off-by: Radostin Stoyanov <[email protected]>
Sort and remove unused/unnecessary include statements in criu/img-*.c

Signed-off-by: Radostin Stoyanov <[email protected]>
The print_data() function was part of the deprecated (and removed)
'show' action, and it was moved in util.c with the following commit:

	a501b48
	The 'show' action has been deprecated since 1.6, let's finally drop it.

	The print_data() routine is kept for yet another (to be deprecated too)
	feature called 'criu exec'.

The criu exec feature was removed with:

	909590a
	Remove criu exec code

	It's now obsoleted by compel library.
	Maybe-TODO: Add compel tool exec action?

Therefore, now we can drop print_data() as well.

Signed-off-by: Radostin Stoyanov <[email protected]>
class ctypes.c_char_p
    Represents the C char * datatype when it points to a zero-
    terminated string. For a general character pointer that may
    also point to binary data, POINTER(c_char) must be used.
    The constructor accepts an integer address, or a bytes object.

https://docs.python.org/3/library/ctypes.html#ctypes.c_char_p

Signed-off-by: Radostin Stoyanov <[email protected]>
There are a few places where spaces have been used instead of tabs for
indentation. This patch converts the spaces to tabs for consistency
with the rest of the code base.

Signed-off-by: Radostin Stoyanov <[email protected]>
criu-3.12/criu/net.c:2043: overwrite_var: Overwriting "img" in "img =
open_image_at(-1, CR_FD_IP6TABLES, 0UL, pid)" leaks the storage that
"img" points to.

Signed-off-by: Adrian Reber <[email protected]>
The function clear_ghost_files() has been removed in commit
b11eeea "restore: auto-unlink for ghost files (v2)".

Signed-off-by: Radostin Stoyanov <[email protected]>
avagin and others added 27 commits April 1, 2020 00:10
This test checks that monotonic and boottime don't jump after C/R.

In ns and uns flavors, the test is started in a separate time namespace
with big offsets, so if criu will restore a time namespace incorrectly
the test will detect the big delta of clocks values before and after C/R.

Signed-off-by: Andrei Vagin <[email protected]>
After restoring processes, we have to be sure that monotonic and
boottime clocks will not go backward. For this, we can restore processes
in a new time namespace and set proper offsets for the clocks.

In this patch, criu dumps clocks values event when processes are running
in this host time namespace and on restore, criu creates a new time
namespace, sets dumped clock values and restores processes.

Signed-off-by: Andrei Vagin <[email protected]>
By default, this limit is 80 symbols and this isn't enough:
 4730 pts/0    S+     0:00          \_ ./zdtm_ct zdtm.py
7535 4731 pts/0    S+     0:00          |   \_ python zdtm.py
7536 4839 pts/0    S+     0:00          |       \_ python zdtm.p
7537 4861 pts/0    S+     0:00          |           \_ make --no
7538 4882 pts/0    S+     0:00          |               \_ ./mnt
7539 4883 ?        Ss     0:00          |                   \_ .

Signed-off-by: Andrei Vagin <[email protected]>
…braries

     This patch only adds the support but does not enable it for building.

Signed-off-by: Guoyun Sun <[email protected]>
Before, reaching EOF too soon would trigger an infinite loop.

Signed-off-by: Nicolas Viennot <[email protected]>
Oddly, one of the test had a typo which should be fatal.

Signed-off-by: Nicolas Viennot <[email protected]>
When an image is opened but errored with a ENOENT error, the image is
still valid. Later on, do_pb_read_one() can fail and will invoke
image_name(). The image fd is EMPTY_IMG_FD (-404). read_fd_link fails.

Signed-off-by: Nicolas Viennot <[email protected]>
Reducing our memory footprint by 4K.

Improved-by: Andrei Vagin <[email protected]>
Signed-off-by: Nicolas Viennot <[email protected]>
This commit introduces an optimization when rsti(t)->vma_io is empty.
This optimization allows streaming a non-seekable image as CR_FD_PAGES
is not reopened.

Signed-off-by: Nicolas Viennot <[email protected]>
In real life cases pipe_ino param could be larger that INT_MAX,
but in autofs_parse() function we using atoi function, that uses
4 byte integers. It's a bug.

Example of mount info from real case:
(00.508286) 	type autofs source /etc/auto.misc mnt_id 2824 s_dev 0x4b9 / @
./misc flags 0x300000 options fd=5,pipe_ino=3480845226,pgrp=95929,timeout=300,
minproto=5,maxproto=5,indirect

3480845226 > 2147483647 (32-bit wide signed int max value) => we have a problem

It causes a error:
(03.195915) Error (criu/pipes.c:529): The packetized mode for pipes is not supported yet

Signed-off-by: Alexander Mikhalitsyn (Virtuozzo) <[email protected]>
We should follow Linux Kernel Codding Style:

... the closing brace is empty on a line of its own, except in the cases
where it is followed by a continuation of the same statement, ie ... an
else in an if-statement ...

https://www.kernel.org/doc/html/v4.10/process/coding-style.html#placing-braces-and-spaces

Automaticly fixing with:

:!git grep --files-with-matches "^\s*else[^{]*{" | xargs
:argadd <files>
:argdo :%s/}\s*\n\s*\(else[^{]*{\)/} \1/g | update

Signed-off-by: Pavel Tikhomirov <[email protected]>
kerndat_socket_netns() is called twice. We keep the latter to avoid
changing the behavior.

Signed-off-by: Nicolas Viennot <[email protected]>
Changed all the %u into %d.

Ideally, we should implement the %u format for parasite code.

Signed-off-by: Nicolas Viennot <[email protected]>
Func kerndat_nsid() is called twice.

v2: leave kerndat_nsid call near kerndat_link_nsid

Signed-off-by: Pavel Tikhomirov <[email protected]>
First don't free pstree_item as they are allocated with shmalloc on
restore. Second always pstree_entry__free_unpacked PstreeEntry. Third
remove all breaks replacing them with implict goto err, so that it would
be easier to understand that we are on error path. Forth split out
code for reading one pstree item in separate function.

Sadly there is no much use in xfree-ing pi->threads because in case of
an error we still have ->threads unfreed from previous entries anyway.

But at least some cleanup can be done here.

Signed-off-by: Pavel Tikhomirov <[email protected]>
Clock IDs in this file has been replaced by clock symbolic names.

Now it looks like this:
    $ cat /proc/774/timens_offsets
    monotonic      864000         0
    boottime      1728000         0

Signed-off-by: Andrei Vagin <[email protected]>
We permanently have issues like this:
./test/jenkins/criu-iter.sh: 3: source: not found

It looks like a good idea to use one shell to run our jenkins scripts.

Signed-off-by: Andrei Vagin <[email protected]>
When testing runc checkpointing, I frequently see the following error:

> Error (criu/mount.c:1107): mnt: Can't create a temporary directory: Read-only file system

This happens because container root is read-only mount.

The error here is not actually fatal since it is handled later
in ns_open_mountpoint() (at least since [1] is fixed), but it is shown
as error in runc integration tests.

Since it is not fatal, let's demote it to a warning to avoid confusion.

[1] checkpoint-restore#520

Signed-off-by: Kir Kolyshkin <[email protected]>
…task is yet to be completed: parsing BPF file info from the proc filesystem in the function parse_fdinfo_pid_s()
@avagin
Copy link
Member

avagin commented May 8, 2020

#1044

@avagin avagin closed this May 8, 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.