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

Try packit 2 #6

Open
wants to merge 204 commits into
base: criu-dev
Choose a base branch
from
Open

Try packit 2 #6

wants to merge 204 commits into from

Conversation

adrianreber
Copy link
Owner

Signed-off-by: Adrian Reber areber@redhat.com

xemul and others added 30 commits November 8, 2020 01:55
So, here's the enhanced version of the first try.

Changes are:

1. The wrapper name is criu-ns instead of crns.py
2. The CLI is absolutely the same as for criu, since the script
   re-execl-s criu binary. E.g.
	   scripts/criu-ns dump -t 1234 ...
   just works
3. Caller doesn't need to care about substituting CLI options,
   instead, the scripts analyzes the command line and
   a) replaces -t|--tree argument with virtual pid __if__ the
      target task lives in another pidns
   b) keeps the current cwd (and root) __if__ switches to another
      mntns. A limitation applies here -- cwd path should be the
      same in target ns, no "smart path mapping" is performed. So
      this script is for now only useful for mntns clones (which
      is our main goal at the moment).

Signed-off-by: Pavel Emelyanov <xemul@virtuozzo.com>
Looks-good-to: Andrey Vagin <avagin@openvz.org>
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 <rstoyanov1@gmail.com>
Signed-off-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
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 <rstoyanov1@gmail.com>
Change made through this commit:
- Include copy of flog as a seperate tree.
- Modify the makefile to add and compile flog code.

Signed-off-by: prakritigoyal19 <prakritigoyal19@gmail.com>
CID 302713 (#1 of 1): Missing varargs init or cleanup (VARARGS)
 va_end was not called for argptr.

Signed-off-by: Adrian Reber <areber@redhat.com>
criu-ns performs double fork, which results in criu restore
using PID=2. Thus, if a user is trying to restore a process
with that PID, the restore will fail.

Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
When criu restore runs as PID=1 it has an additional responsibility to
reap zombie processes.

Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Signed-off-by: Andrei Vagin <avagin@gmail.com>
Some kernels have W^X mitigation, which means they won't execute memory
blocks if that memory block is also writable or ever was writable. This
patch enables CRIU to run on such kernels.

1. Align .data section to a page.
2. mmap a memory block for parasite as RX.
3. mprotect everything after .text as RW.

Signed-off-by: Michał Cłapiński <mclapinski@google.com>
The clang analyzer, scan-build, cannot correctly handle the
LOCK_BUG_ON() macro. At multiple places there is the following warning:

  Error: CLANG_WARNING:
    criu/pie/restorer.c:1221:4: warning: Dereference of null pointer

  include/common/lock.h:14:35: note: expanded from macro 'LOCK_BUG_ON'
               *(volatile unsigned long *)NULL = 0xdead0000 + __LINE__
               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~

This just disable the clang analyzer for the LOCK_BUG_ON() macro.

Signed-off-by: Adrian Reber <areber@redhat.com>
Using scan-build there is a warning about

 infect.c:231:17: warning: The left operand of '!=' is a garbage value
                 if (ss->state != 'Z') {

which is a false positive as every process will have a 'Status' field,
but initializing the structure makes the clang analyzer silent.

Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
The function collect_one_unixsk() has a parameter 'i' and at the same
time has a variable, in a loop, with the name 'i'.

This is no real error or problem, because the function parameter 'i' is
never used in the whole function.

Just trying to reduce confusion and making a code checker happy.

Signed-off-by: Adrian Reber <areber@redhat.com>
Using strsep() moves the pointer of the original string and this
introduces a copy of the malloc()ed memory to be able to free() it
later.

Signed-off-by: Adrian Reber <areber@redhat.com>
Ignore coverity errors about CHECKED_RETURN.

Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Coverity does not understand how close_fds() works.

Signed-off-by: Adrian Reber <areber@redhat.com>
…ative

Signed-off-by: Adrian Reber <areber@redhat.com>
criu/sk-unix.c:1173: chroot_call: Calling chroot: "chroot(".")".
criu/sk-unix.c:1175: chroot: Calling function "close_safe" after chroot() but before calling chdir("/").

criu/sk-unix.c:1251: chroot_call: Calling chroot: "chroot(".")".
criu/sk-unix.c:1263: chroot: Calling function "print_on_level" after chroot() but before calling chdir("/").

Coverity also says:

175312, 175313 Insecure chroot

If a call to chroot is not followed by a call to chdir("/") the chroot jail confinement can be violated.

Signed-off-by: Adrian Reber <areber@redhat.com>
criu/pagemap.c:245: negative_return_fn: Function "img_raw_fd(pr->pi)" returns a negative number.
criu/pagemap.c:245: assign: Assigning: "fd" = "img_raw_fd(pr->pi)".
criu/pagemap.c:258: negative_returns: "fd" is passed to a parameter that cannot be negative.

criu/ipc_ns.c:762: negative_return_fn: Function "img_raw_fd(img)" returns a negative number.
criu/ipc_ns.c:762: assign: Assigning: "ifd" = "img_raw_fd(img)".
criu/ipc_ns.c:768: negative_returns: "ifd" is passed to a parameter that cannot be negative.

Signed-off-by: Adrian Reber <areber@redhat.com>
criu/namespaces.c:529: var_decl: Declaring variable "path" without initializer.
criu/namespaces.c:602: uninit_use_in_call: Using uninitialized value "*path" as argument to "%s" when calling "print_on_level".

Signed-off-by: Adrian Reber <areber@redhat.com>
CRIU is already using multiple CI systems and not just Travis. This
renames all Travis related things to 'ci' to show it is actually
independent of Travis.

Just a simple rename.

Signed-off-by: Adrian Reber <areber@redhat.com>
@adrianreber
Copy link
Owner Author

I'm afraid aarch64 is not yet supported architecture in tests. @thrix would tell you more.
I'll try to improve the error message (Bad Request).

Thanks, I will remove it.

Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
jpopelka added a commit to jpopelka/packit-service that referenced this pull request Apr 15, 2021
When a TF refuses a new test request, it sends back
reason: 'Bad Request'
errors: (e.g.) {'environments': {'0': {'arch': "must be one of: 'x86_64'"}}}

While the reason is nicer, it doesn't contain any info
so users would need to wait for us to tell them what
actually happened.

So far we've hit only the unsupported arch error,
so let's extract it from the error json.

adrianreber/criu#6 (comment)
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
@thrix
Copy link

thrix commented Apr 15, 2021

I'm afraid aarch64 is not yet supported architecture in tests. @thrix would tell you more.
I'll try to improve the error message (Bad Request).

Yeah, not yet ... I fillled: https://gitlab.com/testing-farm/general/-/issues/23

Signed-off-by: Adrian Reber <areber@redhat.com>
jpopelka added a commit to jpopelka/packit-service that referenced this pull request Apr 15, 2021
When a TF refuses a new test request, it sends back
reason: 'Bad Request'
errors: (e.g.) {'environments': {'0': {'arch': "must be one of: 'x86_64'"}}}

While the reason is nicer, it doesn't contain any info
so users would need to wait for us to tell them what
actually happened.

So far we've hit only the unsupported arch error,
so let's extract it from the error json.

adrianreber/criu#6 (comment)
Signed-off-by: Adrian Reber <areber@redhat.com>
softwarefactory-project-zuul bot added a commit to packit/packit-service that referenced this pull request Apr 15, 2021
Error message is in 'errors', not 'message'

When a TF refuses a new test request, it sends back
reason: 'Bad Request'
errors: (e.g.) {'environments': {'0': {'arch': "must be one of: 'x86_64'"}}}
While the reason is nicer, it doesn't contain any info so users would need to wait for us to tell them what actually happened.
So far we've hit only the unsupported arch error, so let's extract it from the error json.
adrianreber/criu#6 (comment)

Reviewed-by: None <None>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
@TomasTomecek
Copy link

Congrats on getting green tests, you rock!

@psss
Copy link

psss commented Apr 19, 2021

Just note, if you don't plan to enable multiple tests it's also possible to include the whole config within the plan:

environment:
    CD_TO_TOP: 1
    SKIP_CI_PREP: 1
    CC: gcc
prepare:
    script:
      - ./scripts/ci/prepare-for-fedora-rawhide.sh
      - dnf install -y libselinux-devel
      - systemctl stop sssd
execute:
    script:
      - ln -sf /usr/include/google/protobuf/descriptor.proto images/google/protobuf/descriptor.proto
      - ./scripts/ci/run-ci-tests.sh

Scripts defined directly under the execute step are run in the git repo root so the CD_TO_TOP variable would not be needed.

@adrianreber
Copy link
Owner Author

@psss Thanks. I will try that. That looks simpler and easier to read.

@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.