<div dir="ltr">Yes, there's definitely a bug. Filipe was suggesting that perhaps the prctl() calls are failing not due to lack of support in the kernel but due to some other error (e.g., passing wrong parameters). This causes "criu check" to think the old API is not supported but in reality it is and that's why c/r works.<div><br></div><div>Below is a complete log showing all your patches applied. Hope this helps!</div><div><br></div><div>--Saied</div><div><br></div><div><br></div><div><div>$ uname -r</div><div>3.13.0-77-generic</div><div><br></div><div><div>$ git log -n 3 --oneline</div><div>6576008 check: Call sbrk to get current brk</div><div>7ae34a7 check: Use errno to check for prctl error code (for exe file)</div><div>e37fc4f check: Use errno to check for prctl error code (for brk setting)</div><div><br></div><div>$ make clean</div><div>...</div><div><br></div><div>$ make -j 12</div><div>...</div><div><br></div><div>$ sudo ./criu/criu check </div><div>prctl: PR_SET_MM_MAP is not supported, which is required for restoring user namespaces</div><div>Error (cr-check.c:629): Kernel doesn't support PTRACE_O_SUSPEND_SECCOMP</div><div>Error (cr-check.c:673): Dumping seccomp filters not supported: Input/output error</div><div>Error (timerfd.c:55): timerfd: No timerfd support for c/r: Inappropriate ioctl for device</div><div>Error (cr-check.c:307): fdinfo doesn't contain the mnt_id field</div><div>Error (cr-check.c:770): AIO remap doesn't work properly</div><div>Error (cr-check.c:786): fdinfo doesn't contain the lock field</div><div>Error (cr-check.c:833): cgroupns not supported. This is not fatal.</div><div><br></div><div>$ echo $?</div><div>1</div><div><br></div><div>$ sudo ./criu/criu check --ms</div><div>Warn (cr-check.c:195): Skipping unssuported PR_SET_MM_MAP</div><div>Error (cr-check.c:629): Kernel doesn't support PTRACE_O_SUSPEND_SECCOMP</div><div>Error (cr-check.c:673): Dumping seccomp filters not supported: Input/output error</div><div>Error (timerfd.c:55): timerfd: No timerfd support for c/r: Inappropriate ioctl for device</div><div>Error (cr-check.c:307): fdinfo doesn't contain the mnt_id field</div><div>Warn (cr-check.c:773): Skipping unsupported AIO remap</div><div>Warn (cr-check.c:789): fdinfo doesn't contain the lock field</div><div>Warn (cr-check.c:827): Skipping cgroup namespaces check</div><div><br></div><div>$ echo $?</div><div>1</div><div><br></div><div>$ ../a.out &</div><div>[1] 21409</div><div><br></div><div>$ sudo ./criu/criu dump -D /tmp/criu -o dump.log -v4 -j -t 21409</div><div>[1]+ Killed ../a.out</div><div><br></div><div>$ echo $?</div><div>0</div><div><br></div><div>$ sudo ./criu/criu restore -D /tmp/criu -o retore.log -v4 -j -t 21409 -d</div><div><br></div><div>$ echo $?</div><div>0</div><div><br></div><div>$ ps -efl | grep a.out</div><div>5 R saied 21409 1 99 80 0 - 1047 - 16:05 pts/0 00:00:06 ../a.out</div><div>0 S saied 21431 8198 0 80 0 - 5938 pipe_w 16:05 pts/0 00:00:00 grep --color=auto a.out</div></div></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 14, 2016 at 3:20 PM, Pavel Emelyanov <span dir="ltr"><<a href="mailto:xemul@virtuozzo.com" target="_blank">xemul@virtuozzo.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 03/14/2016 08:35 PM, Saied Kazemi wrote:<br>
<span class="">><br>
><br>
> On Mon, Mar 14, 2016 at 1:22 AM, Pavel Emelyanov <<a href="mailto:xemul@virtuozzo.com">xemul@virtuozzo.com</a> <mailto:<a href="mailto:xemul@virtuozzo.com">xemul@virtuozzo.com</a>>> wrote:<br>
><br>
> On 03/12/2016 01:53 AM, Saied Kazemi wrote:<br>
> ><br>
> ><br>
</span><div><div class="h5">> > On Fri, Mar 11, 2016 at 5:04 AM, Pavel Emelyanov <<a href="mailto:xemul@virtuozzo.com">xemul@virtuozzo.com</a> <mailto:<a href="mailto:xemul@virtuozzo.com">xemul@virtuozzo.com</a>> <mailto:<a href="mailto:xemul@virtuozzo.com">xemul@virtuozzo.com</a> <mailto:<a href="mailto:xemul@virtuozzo.com">xemul@virtuozzo.com</a>>>> wrote:<br>
> ><br>
> > On 03/11/2016 12:01 AM, Saied Kazemi wrote:<br>
> > > Pavel and Adrian,<br>
> > ><br>
> > > Please take a look at this patch and let me know what you think.<br>
> > ><br>
> > > As you will see, I have changed some pr_error() to pr_warn() when calling<br>
> > > "criu check --extra" because we're saying that the extra features do not<br>
> > > necessarily cause dump/restore failures. But we can't change all pr_error()<br>
> > > calls to pr_warn() indiscriminately in the functions that are called by<br>
> > > "criu check --extra" because some of them are real errors.<br>
> ><br>
> > Well, yes. If you're talking about pr_perror()-s that are called by non cr-check.c<br>
> > code, we can shut this down by manually lowering the log level.<br>
> ><br>
> > As far as such replacements in cr-check.c, this seem to be related to which return<br>
> > code we want to have when extra features checks fail.<br>
> ><br>
> > > Another item of discussion is the exit code. Obviously, if there are no<br>
> > > errors or warnings the exit code has to be zero. But what if there's a<br>
> > > warning on an extra or specific feature?<br>
> > > Currently, the exit code is non-zero but one can argue that for warnings it<br>
> > > should still be zero.<br>
> ><br>
> > I would say it should be non-zero, as user _explicitly_ asks to check for extra<br>
> > feature, and if it's absent, then the check has failed. And, respectively, the<br>
> > pr_err/_perror should probably remain such.<br>
> ><br>
> > > The current patch is a definite improvement (because it was broken) but to make<br>
> > > it "perfect" will require more time and iterations. It's also very difficult to<br>
> > > test all permutations.<br>
> ><br>
> > I like the patch and want to merge it, but have only one comment, please, find it<br>
> > inline:<br>
> ><br>
> ><br>
> > > @@ -182,38 +183,13 @@ static int check_prctl(void)<br>
> > > return -1;<br>
> > > }<br>
> > ><br>
> > > - /*<br>
> > > - * Either new or old interface must be supported in the kernel.<br>
> > > - */<br>
> > > - ret = prctl(PR_SET_MM, PR_SET_MM_MAP_SIZE, (unsigned long)&size, 0, 0);<br>
> > > - if (ret) {<br>
> > > - if (!opts.check_ms_kernel) {<br>
> > > - pr_msg("prctl: PR_SET_MM_MAP is not supported, which "<br>
> > > - "is required for restoring user namespaces\n");<br>
> > > - return -1;<br>
> > > - } else<br>
> > > - pr_warn("Skipping unssuported PR_SET_MM_MAP\n");<br>
> > > -<br>
> > > - ret = prctl(PR_SET_MM, PR_SET_MM_BRK, brk(0), 0, 0);<br>
> > > - if (ret) {<br>
> > > - if (ret == -EPERM)<br>
> > > - pr_msg("prctl: One needs CAP_SYS_RESOURCE capability to perform testing\n");<br>
> > > - else<br>
> > > - pr_msg("prctl: PR_SET_MM is not supported\n");<br>
> > > - return -1;<br>
> > > - }<br>
> > > -<br>
> > > - ret = prctl(PR_SET_MM, PR_SET_MM_EXE_FILE, -1, 0, 0);<br>
> > > - if (ret != -EBADF) {<br>
> > > - pr_msg("prctl: PR_SET_MM_EXE_FILE is not supported (%d)\n", ret);<br>
> > > - return -1;<br>
> > > - }<br>
> > > -<br>
> > > - ret = prctl(PR_SET_MM, PR_SET_MM_AUXV, (long)&user_auxv, sizeof(user_auxv), 0);<br>
> > > - if (ret) {<br>
> > > - pr_msg("prctl: PR_SET_MM_AUXV is not supported\n");<br>
> > > - return -1;<br>
> > > - }<br>
> ><br>
> > The checks for PR_SET_MM_BRK, _EXE_FILE and _AUXV are still necessary, since they<br>
> > were added exclusively for CRIU. And CRIU still calls this stuff actually (in user<br>
> > mode), so presence of these things should be left in Category 1.<br>
> ><br>
> > -- Pavel<br>
> ><br>
> ><br>
> > This is a bit confusing. On my system (running 3.13 kernel) criu freshly created from the head<br>
> > will fail both "criu check" and "criu check --ms" which means I do not have either the new API<br>
> > (prctl(PR_SET_MM, PR_SET_MM_MAP_SIZE, ...)) or the old API (prctl(PR_SET_MM, PR_SET_MM_BRK, ...)).<br>
> > However, I can successfully dump and restore a simple main() { while (1); } C binary.<br>
><br>
> Hm... That's because we have a number of bugs in there :( I'll send some patches soon.<br>
><br>
><br>
> The patches fix the bugs where ret was confused with errno. But even with those patches,<br>
> both "criu check" and "criu check --ms" fail while I can still successfully dump and restore<br>
> the loop process.<br>
<br>
</div></div>In my 3 patches set there was patch #3, that was not in Filipe's patch -- it<br>
fixes the brk() to be sbrk(). Would you check whether this patch helps? And if<br>
no -- would you show the criu check with and without --ms output?<br>
<span class=""><br>
> My expectation is that if "criu check ]--ms]" fails, I should not be able to dump and restore.<br>
<br>
</span>Yes, there's something weird with these prctls :\<br>
<span class=""><br>
> > Why PR_SET_MM_BRK, _EXE_FILE and _AUXV are absolutely required as Category 1 features?<br>
><br>
> Because the PR_SET_MM_MAP_SIZE was introduced in 3.18 (or 3.17) with<br>
> <a href="https://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=f606b77f1" rel="noreferrer" target="_blank">https://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=f606b77f1</a><br>
><br>
><br>
> This explains why PR_SET_MM_MAP_SIZE was introduced and how it works. But pertaining to<br>
> my comment above where I can successfully c/r, it does not explain why it has to be a Category<br>
> 1 feature. Sorry if I am missing something obvious.<br>
<br>
</span>Well, for now I bet there's still some bug with this code :)<br>
<span class="HOEnZb"><font color="#888888"><br>
-- Pavel<br>
<br>
</font></span></blockquote></div><br></div>