<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 11, 2016 at 5:04 AM, 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">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>
</span>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>
<span class=""><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>
</span>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>
<span class=""><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>
</span>I like the patch and want to merge it, but have only one comment, please, find it<br>
inline:<br>
<div><div class="h5"><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>
</div></div>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>
<span class=""><font color="#888888"><br>
-- Pavel<br>
<br>
</font></span></blockquote></div><br></div><div class="gmail_extra">This is a bit confusing. On my system (running 3.13 kernel) criu freshly created from the head will fail both "criu check" and "criu check --ms" which means I do not have either the new API (prctl(PR_SET_MM, PR_SET_MM_MAP_SIZE, ...)) or the old API (prctl(PR_SET_MM, PR_SET_MM_BRK, ...)). However, I can successfully dump and restore a simple main() { while (1); } C binary.</div><div class="gmail_extra"><br></div><div class="gmail_extra">Why PR_SET_MM_BRK, _EXE_FILE and _AUXV are absolutely required as Category 1 features?</div><div class="gmail_extra"><br></div><div class="gmail_extra">--Saied</div><div class="gmail_extra"><br></div></div>