<div dir="ltr">I am unfortunately leaving back for school at the end of this week so my ability to further contribute will be limited.<div><br></div><div>If we want a failure in the cgroup property restore to stop the entire restoration, we need a patch further up the call chain. I agree the behavior should either be a complete stop of the restore or the behavior supplied by the above patch.</div>
<div><br></div><div>While it seems there are different cases where we want different behaviors when properties are missing on the restore side, I think we are all in agreement that properties that are missing during dump should just be ignored. Would you like me to break this patch into 2 patches, handling missing property failure on the dump side and the restore side, so we can merge the dump patch? Getting this part merged is a priority for me as it blocking some of my colleagues.</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Aug 14, 2014 at 7:15 AM, Pavel Emelyanov <span dir="ltr">&lt;<a href="mailto:xemul@parallels.com" target="_blank">xemul@parallels.com</a>&gt;</span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 08/14/2014 06:01 PM, Serge Hallyn wrote:<br>
&gt; Quoting Pavel Emelyanov (<a href="mailto:xemul@parallels.com">xemul@parallels.com</a>):<br>
&gt;&gt; On 08/14/2014 01:46 AM, Garrison Bellack wrote:<br>
&gt;&gt;&gt; On Wed, Aug 13, 2014 at 1:55 PM, Tycho Andersen &lt;<a href="mailto:tycho.andersen@canonical.com">tycho.andersen@canonical.com</a> &lt;mailto:<a href="mailto:tycho.andersen@canonical.com">tycho.andersen@canonical.com</a>&gt;&gt; wrote:<br>

&gt;&gt;&gt;<br>
&gt;&gt;&gt;     On Wed, Aug 13, 2014 at 01:32:01PM -0700, Garrison Bellack wrote:<br>
&gt;&gt;&gt;     &gt; On Wed, Aug 13, 2014 at 12:43 PM, Andrew Vagin &lt;<a href="mailto:avagin@parallels.com">avagin@parallels.com</a> &lt;mailto:<a href="mailto:avagin@parallels.com">avagin@parallels.com</a>&gt;&gt; wrote:<br>

&gt;&gt;&gt;     &gt;<br>
&gt;&gt;&gt;     &gt; &gt; On Wed, Aug 13, 2014 at 11:59:33AM -0700, <a href="mailto:gbellack@google.com">gbellack@google.com</a> &lt;mailto:<a href="mailto:gbellack@google.com">gbellack@google.com</a>&gt; wrote:<br>
&gt;&gt;&gt;     &gt; &gt; &gt; From: Garrison Bellack &lt;<a href="mailto:gbellack@google.com">gbellack@google.com</a> &lt;mailto:<a href="mailto:gbellack@google.com">gbellack@google.com</a>&gt;&gt;<br>
&gt;&gt;&gt;     &gt; &gt; &gt;<br>
&gt;&gt;&gt;     &gt; &gt; &gt; Because different kernel versions have different cgroup properties, criu<br>
&gt;&gt;&gt;     &gt; &gt; &gt; shouldn&#39;t crash just because the properties statically listed aren&#39;t<br>
&gt;&gt;&gt;     &gt; &gt; exact.<br>
&gt;&gt;&gt;     &gt; &gt; &gt; Instead, during dump, ignore properties the kernel doesn&#39;t have and<br>
&gt;&gt;&gt;     &gt; &gt; continue.<br>
&gt;&gt;&gt;     &gt; &gt; &gt; In addition, during restore, in the event of migration to a kernel that<br>
&gt;&gt;&gt;     &gt; &gt; is<br>
&gt;&gt;&gt;     &gt; &gt; &gt; missing a property that was dumped, print an error but continue.<br>
&gt;&gt;&gt;     &gt; &gt;<br>
&gt;&gt;&gt;     &gt; &gt; I am not sure that we can continue in this case. Why do you think that<br>
&gt;&gt;&gt;     &gt; &gt; it&#39;s safe? I think we need to ask a user about this. So can we add a new<br>
&gt;&gt;&gt;     &gt; &gt; option to allow continuing in this case?<br>
&gt;&gt;&gt;     &gt;<br>
&gt;&gt;&gt;     &gt;<br>
&gt;&gt;&gt;     &gt; I&#39;m a little confused about your concerns of safety. Are you referring to<br>
&gt;&gt;&gt;     &gt; missing properties on the dump or restore side?<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt;     I think restore side. I also wondered the same thing -- it seems that<br>
&gt;&gt;&gt;     missing properties on restore should be an error (perhaps as Andrew<br>
&gt;&gt;&gt;     says with an --allow-missing-cg-props flag on the restore side to<br>
&gt;&gt;&gt;     allow you to override this error).<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt; Here is the reasoning behind this decision. Let say you are migrating properties A,B,C,D where B doesn&#39;t exist.<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt; This is how it currently works:<br>
&gt;&gt;&gt; A will be restored. B prints an error and causes failure because it is missing. However, because cgroup property restoration is the last thing to happen during restore, and because of designs further up the call chain, the restoration of the process still goes through. C,D are not restored because of previous criu failure.<br>

&gt;&gt;&gt; End result -- Process and property A restored, C and D are not restored even though they exist on the new machine<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt; With this patch:<br>
&gt;&gt;&gt; A will be restored. B prints an error and continues. C and D are restored.<br>
&gt;&gt;&gt; End result --  Process and property A, C, D all restored.<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt; I think between these two options we clearly prefer the later behavior.<br>
&gt;&gt;<br>
&gt;&gt; Absolutely. AFAIU Andrew and Tycho meant, that there could be the 3rd behavior -- once B<br>
&gt;&gt; fails we abort all the restore. And they proposed an option for controlling this.<br>
&gt;&gt;<br>
&gt;&gt; I have a question about this -- what does LXC tool do if it meets a cgroup configuration<br>
&gt;&gt; parameter in container&#39;s config, that is missing in the kernel?<br>
&gt;<br>
&gt; Container start will fail.  The difference is that if there is a cgroup<br>
&gt; limit in the container&#39;s configuration file, then it was specifically asked<br>
&gt; for.  Ideally for a criu restart, we want the same behavior (overrideable),<br>
&gt; but we want limits which were not asked for to be ignored.<br>
<br>
</div></div>OK, so we need some fine-grained control over what to do with properties.<br>
And since we&#39;ve started talking about this. In OpenVZ kernel we have much<br>
more cgroup parameters, than those currently listed in CRIU (e.g. kernel<br>
memory control). In the future we will likely have some of them upstream,<br>
and definitely will have some completely new. Patching criu every time<br>
just to add new properties seems quite inconvenient. Especially, taking<br>
into account that this is just the list of strings.<br>
<br>
Can we teach the get_known_properties() to accept lists from the caller<br>
and use similar lists on restore to specify which of them can be ignored?<br>
<br>
Thoughts?<br>
<div class=""><br>
&gt; I.e. if I&#39;ve checkpointed a nested container which has no cpuset specified,<br>
&gt; and the parent container had a cpuset limit specified (say cpu 0), then if I<br>
&gt; restart under another parent container which has a different cpuset (say<br>
&gt; cpu 1), then restart shouldn&#39;t fail, bc that limit was not inherent to<br>
&gt; the container I checkpointed.<br>
<br>
</div>Thanks,<br>
Pavel<br>
<br>
</blockquote></div><br></div>