Re: [PATCH 00/30] C/R OpenVZ/Virtuozzo style



On Tue, Apr 14, 2009 at 01:46:36AM -0400, Oren Laadan wrote:
Some meta comments about this patch set:

* Patches 1-9 are cleanups, unrelated to checkpoint/restart. They
deserve a separate thread.

They will be sent separatedly.

* You barely take locks or reference counts to objects that you
later refer to. What if something really bad happens ?

I'm thinking right now what is needed and isn't.

Tasks are frozen so unmap VMA in the middle of dump can't happen.

Refcounts aren't needed because tasks are frozen but alive and pin
everything needed. This is during dump part.

What is needed is impossibility of a task to dissapear in frozen state.

On restart, I probably leak refcount to every object, but haven't
checked just it.

* (contd) If you don't take locks, then you at the very least need
to rely on the container remaining frozen for the duration of the
operation (during checkpoint).

* (contd) Still with locks and references, during restart you can't
even freeze the container, so need extra logic to prevent bad things
(e.g. OOM killer, signal or ptrace from parent container etc).

* What is the rationale behind doing the freeze/unfreeze from within
sys_checkpoint/sys_restart ? Instead of you let userspace do it
(and only verify in kernel) you gain, again, flexibility. For example,
you want to also snapshot the filesystem, then userspace will do
something like: freeze container -> snapshot filesystem -> checkpoint
-> thaw container.

This is incomplete part. But, yes, freeze, dump, thaw/kill as separate
actions make sense.

checkpoint(CR_CPT_FREEZE);
[rsync fs]
checkpoint(CR_CPT_DUMP|CR_CPT_KILL);

with check that CR_CPT_THAW doesn't happen during dump.

* A plethora of "FIXME" comments ...

Alexey Dobriyan wrote:
This is to show how we see C/R and to provoke discussion on number of
important issues (mounts, ...).

Quoting your patch:
---
This is one big FIXME:
What to do with overmounted files?
What to do with mounts at all, who should restore them?

just restore something to not oops on task exit

This is just a skeleton for mnt_ns checkpoint, not seriously suggested
to merge it.

This is small part of long-awaited to be cleanuped code.

It's able to restore busyloop on i386 and x86_64 and restore i386
busyloop on x86_64. It wasn't tested much more than that.

Oh .. I really wish you'd sent a x86_64 patch our way, too ;)


I'm currently starting formal testsuite, otherwise it's whack-a-mole
game and formal TODO list (a huge one).


So I'm still struggling to see the major different in the approaches
that would justify throwing away our hard worked, reviewed, tested
and functional code, and take this - similar in design, largely
incomplete and unreviewed code.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



Relevant Pages

  • [RFC v2][PATCH 1/9] kernel based checkpoint-restart
    ... Security (without CAPS_SYS_ADMIN files restore may fail) ... all the stakeholders was that doing checkpoint/restart in the kernel ... The checkpoint and restore are done ... container, this patch has no notion of a container. ...
    (Linux-Kernel)
  • Re: [RFC v11][PATCH 00/13] Kernel based checkpoint/restart
    ... Patches 12 and 13 are newer, adding support for c/r of multiple ... Extend checkpoint header with archtiecture dependent header ... Memory restore now maps user pages explicitly to copy data into them, ... all the stakeholders was that doing checkpoint/restart in the kernel ...
    (Linux-Kernel)
  • [RFC v7][PATCH 0/9] Kernel based checkpoint/restart
    ... I'm re-posting this with a couple of fixes: ... but please ignore them and look at the kernel ... Memory restore now maps user pages explicitly to copy data into them, ... The checkpoint and restore are done ...
    (Linux-Kernel)
  • [RFC v8][PATCH 0/12] Kernel based checkpoint/restart
    ... Support "external" checkpoint ... Fix save/restore state of FPU ... Memory restore now maps user pages explicitly to copy data into them, ... all the stakeholders was that doing checkpoint/restart in the kernel ...
    (Linux-Kernel)
  • Re: [Devel] [RFC v8][PATCH 0/12] Kernel based checkpoint/restart
    ... Support "external" checkpoint ... Fix save/restore state of FPU ... Memory restore now maps user pages explicitly to copy data into them, ... all the stakeholders was that doing checkpoint/restart in the kernel ...
    (Linux-Kernel)