Quoting Dan Smith ([email protected]):
> This patch adds a "phase" of checkpoint that saves out information about any
> namespaces the task(s) may have.  Do this by tracking the namespace objects
> of the tasks and making sure that tasks with the same namespace that follow
> get properly referenced in the checkpoint stream.
> 
> I tested this with single and multiple task restore, on top of Oren's
> v13 tree.
> 
> Changes:
>   - Remove the kernel restore path
>   - Punt on nested namespaces
>   - Use __NEW_UTS_LEN in nodename and domainname buffers
>   - Add a note to Documentation/checkpoint/internals.txt to indicate where
>     in the save/restore process the UTS information is kept
>   - Store (and track) the objref of the namespace itself instead of the
>     nsproxy (based on comments from Dave on IRC)
>   - Remove explicit check for non-root nsproxy
>   - Store the nodename and domainname lengths and use cr_write_string()
>     to store the actual name strings
>   - Catch failure of cr_obj_add_ptr() in cr_write_namespaces()
>   - Remove "types" bitfield and use the "is this new" flag to determine
>     whether or not we should write out a new ns descriptor
>   - Replace kernel restore path
>   - Move the namespace information to be directly after the task
>     information record
>   - Update Documentation to reflect new location of namespace info
>   - Support checkpoint and restart of nested UTS namespaces
> 
> Cc: [email protected]
> Signed-off-by: Dan Smith <[email protected]>
> ---
>  Documentation/checkpoint/internals.txt |    1 +
>  checkpoint/Makefile                    |    1 +
>  checkpoint/checkpoint.c                |   66 ++++++++++++++++++++-
>  checkpoint/objhash.c                   |    7 ++
>  checkpoint/restart.c                   |  101 
> ++++++++++++++++++++++++++++++++
>  include/linux/checkpoint.h             |    1 +
>  include/linux/checkpoint_hdr.h         |   11 ++++
>  7 files changed, 185 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/checkpoint/internals.txt 
> b/Documentation/checkpoint/internals.txt
> index c741b6c..bdd202c 100644
> --- a/Documentation/checkpoint/internals.txt
> +++ b/Documentation/checkpoint/internals.txt
> @@ -17,6 +17,7 @@ The order of operations, both save and restore, is as 
> follows:
>    -> thread state: elements of thread_struct and thread_info
>    -> CPU state: registers etc, including FPU
>    -> memory state: memory address space layout and contents
> +  -> namespace information
>    -> filesystem state: [TBD] filesystem namespace state, chroot, cwd, etc
>    -> files state: open file descriptors and their state
>    -> signals state: [TBD] pending signals and signal handling state
> diff --git a/checkpoint/Makefile b/checkpoint/Makefile
> index 607d864..55c5c3d 100644
> --- a/checkpoint/Makefile
> +++ b/checkpoint/Makefile
> @@ -4,3 +4,4 @@
> 
>  obj-$(CONFIG_CHECKPOINT) += sys.o checkpoint.o restart.o objhash.o \
>               ckpt_mem.o rstr_mem.o ckpt_file.o rstr_file.o
> +EXTRA_CFLAGS += -DDEBUG
> diff --git a/checkpoint/checkpoint.c b/checkpoint/checkpoint.c
> index c2f0e16..5f83e83 100644
> --- a/checkpoint/checkpoint.c
> +++ b/checkpoint/checkpoint.c
> @@ -213,6 +213,65 @@ static int cr_write_tail(struct cr_ctx *ctx)
>       return ret;
>  }
> 
> +static int cr_write_utsns(struct cr_ctx *ctx, struct new_utsname *name)
> +{
> +     struct cr_hdr h;
> +     struct cr_hdr_utsns *hh = cr_hbuf_get(ctx, sizeof(*hh));
> +     int ret;
> +
> +     h.type = CR_HDR_UTSNS;
> +     h.len = sizeof(*hh);
> +
> +     hh->nodename_len = strlen(name->nodename) + 1;
> +     hh->domainname_len = strlen(name->domainname) + 1;
> +
> +     ret = cr_write_obj(ctx, &h, hh);
> +     if (ret < 0)
> +             goto out;
> +
> +     ret = cr_write_string(ctx, name->nodename, hh->nodename_len);
> +     if (ret < 0)
> +             goto out;
> +
> +     ret = cr_write_string(ctx, name->domainname, hh->domainname_len);
> + out:
> +     cr_hbuf_put(ctx, sizeof(*hh));
> +
> +     return ret;
> +}
> +
> +static int cr_write_namespaces(struct cr_ctx *ctx, struct task_struct *t)
> +{
> +     struct cr_hdr h;
> +     struct cr_hdr_namespaces *hh = cr_hbuf_get(ctx, sizeof(*hh));
> +     struct nsproxy *nsp = t->nsproxy;
> +     int ret;
> +     int uts;
> +
> +     h.type = CR_HDR_NS;
> +     h.len = sizeof(*hh);
> +
> +     uts = cr_obj_add_ptr(ctx, nsp->uts_ns, &hh->uts_ref, CR_OBJ_UTSNS, 0);
> +     if (uts < 0)
> +             goto out;
> +
> +     ret = cr_write_obj(ctx, &h, hh);
> +     if (ret)
> +             goto out;
> +
> +     if (uts) {
> +             ret = cr_write_utsns(ctx, &nsp->uts_ns->name);
> +             if (ret < 0)
> +                     goto out;
> +     }
> +
> +     /* FIXME: Write other namespaces here */
> + out:
> +     cr_hbuf_put(ctx, sizeof(*hh));
> +
> +     return ret;
> +}
> +
>  /* dump the task_struct of a given task */
>  static int cr_write_task_struct(struct cr_ctx *ctx, struct task_struct *t)
>  {
> @@ -267,6 +326,10 @@ static int cr_write_task(struct cr_ctx *ctx, struct 
> task_struct *t)
>               goto out;
>       ret = cr_write_cpu(ctx, t);
>       cr_debug("cpu: ret %d\n", ret);
> +     if (ret < 0)
> +             goto out;
> +     ret = cr_write_namespaces(ctx, t);
> +     cr_debug("ns: ret %d\n", ret);
>   out:
>       return ret;
>  }
> @@ -302,9 +365,6 @@ static int cr_may_checkpoint_task(struct task_struct *t, 
> struct cr_ctx *ctx)
>       if (t != current && !frozen(t))
>               return -EBUSY;
> 
> -     if (task_nsproxy(t)->uts_ns != ctx->root_nsproxy->uts_ns)
> -             return -EPERM;
> -
>       if (task_nsproxy(t)->ipc_ns != ctx->root_nsproxy->ipc_ns)
>               return -EPERM;
> 
> diff --git a/checkpoint/objhash.c b/checkpoint/objhash.c
> index 25916c1..c6ae7c1 100644
> --- a/checkpoint/objhash.c
> +++ b/checkpoint/objhash.c
> @@ -12,6 +12,7 @@
>  #include <linux/file.h>
>  #include <linux/hash.h>
>  #include <linux/checkpoint.h>
> +#include <linux/utsname.h>
> 
>  struct cr_objref {
>       int objref;
> @@ -38,6 +39,9 @@ static void cr_obj_ref_drop(struct cr_objref *obj)
>       case CR_OBJ_INODE:
>               iput((struct inode *) obj->ptr);
>               break;
> +     case CR_OBJ_UTSNS:
> +             put_uts_ns((struct uts_namespace *) obj->ptr);
> +             break;
>       default:
>               BUG();
>       }
> @@ -55,6 +59,9 @@ static int cr_obj_ref_grab(struct cr_objref *obj)
>               if (!igrab((struct inode *) obj->ptr))
>                       ret = -EBADF;
>               break;
> +     case CR_OBJ_UTSNS:
> +             get_uts_ns((struct uts_namespace *) obj->ptr);
> +             break;
>       default:
>               BUG();
>       }
> diff --git a/checkpoint/restart.c b/checkpoint/restart.c
> index d9e01ce..f42d549 100644
> --- a/checkpoint/restart.c
> +++ b/checkpoint/restart.c
> @@ -15,6 +15,8 @@
>  #include <linux/magic.h>
>  #include <linux/checkpoint.h>
>  #include <linux/checkpoint_hdr.h>
> +#include <linux/utsname.h>
> +#include <linux/syscalls.h>
> 
>  #include "checkpoint_arch.h"
> 
> @@ -237,6 +239,101 @@ static int cr_read_tail(struct cr_ctx *ctx)
>       return ret;
>  }
> 
> +static int cr_read_utsns(struct cr_ctx *ctx, struct task_struct *t)
> +{
> +     struct cr_hdr_utsns hh;
> +     struct uts_namespace *ns;
> +     int ret;
> +     char *nn = NULL;
> +     char *dn = NULL;
> +
> +     ret = cr_read_obj_type(ctx, &hh, sizeof(hh), CR_HDR_UTSNS);
> +     if (ret < 0)
> +             return ret;
> +
> +     nn = kmalloc(hh.nodename_len, GFP_KERNEL);
> +     if (!nn) {
> +             ret = -ENOMEM;
> +             goto out;
> +     }
> +
> +     dn = kmalloc(hh.domainname_len, GFP_KERNEL);
> +     if (!dn) {
> +             ret = -ENOMEM;
> +             goto out;
> +     }
> +
> +     ret = cr_read_string(ctx, nn, hh.nodename_len);
> +     if (ret < 0)
> +             goto out;
> +
> +     ret = cr_read_string(ctx, dn, hh.domainname_len);
> +     if (ret < 0)
> +             goto out;
> +
> +     ret = sys_unshare(CLONE_NEWUTS);

One thing to note is that this will drive the ns cgroup
bananas.  It might still be worthwhile collecting the
flags for all the to-be-unshared namespaces, and then
doing all of the unsharing at once.

Futhermore, you do sys_unshare here, then further down you
do another copy_namespaces(CLONE_NEWUTS)?

Finally, it seems to me every task will unshare(CLONE_NEWUTS),
no?  Where is the check done (and stored) for whether this
task has a different utsns from its parent?

I could be misunderstanding your code...

But it seems to me a simpler algorith would be:

Save identifiers for all of the namespaces at the top of the
checkpoint image;  have restart create a set of dummy tasks,
enough to contain all of the new namespaces;  have each unshare
their  namespaces;  then, as each real new task is restarted,
manually create a new nsproxy and link it to all of the
required new namespaces.

OR you can stick to trying to use clone(), but I don't think
this patch is doing that right.

> +     if (ret)
> +             goto out;
> +
> +     ns = t->nsproxy->uts_ns;
> +     memcpy(ns->name.nodename, nn, hh.nodename_len);
> +     memcpy(ns->name.domainname, dn, hh.domainname_len);
> +
> + out:
> +     kfree(nn);
> +     kfree(dn);
> +
> +     return ret;
> +}
> +
> +static int cr_restore_utsns(struct cr_ctx *ctx, int ref)
> +{
> +     struct uts_namespace *uts;
> +     int ret;
> +
> +     uts = cr_obj_get_by_ref(ctx, ref, CR_OBJ_UTSNS);
> +     if (uts == NULL) {
> +             ret = cr_read_utsns(ctx, current);
> +             if (ret < 0)
> +                     return ret;
> +
> +             return cr_obj_add_ref(ctx, current->nsproxy->uts_ns,
> +                                   ref, CR_OBJ_UTSNS, 0);
> +     } else if (IS_ERR(uts)) {
> +             cr_debug("Failed to get UTS ns from objhash");
> +             return PTR_ERR(uts);
> +     }
> +
> +     ret = copy_namespaces(CLONE_NEWUTS, current);
> +     if (ret < 0)
> +             return ret;
> +
> +     put_uts_ns(current->nsproxy->uts_ns);
> +     get_uts_ns(uts);
> +     current->nsproxy->uts_ns = uts;
> +
> +     return 0;
> +}
> +
> +static int cr_read_namespaces(struct cr_ctx *ctx)
> +{
> +     struct cr_hdr_namespaces hh;
> +     int ret;
> +
> +     ret = cr_read_obj_type(ctx, &hh, sizeof(hh), CR_HDR_NS);
> +     if (ret < 0)
> +             return ret;
> +
> +     ret = cr_restore_utsns(ctx, hh.uts_ref);
> +     cr_debug("uts ns: %d\n", ret);
> +     if (ret < 0)
> +             return ret;
> +
> +     /* FIXME: Add more namespaces here */
> +
> +     return 0;
> +}
> +
>  /* read the task_struct into the current task */
>  static int cr_read_task_struct(struct cr_ctx *ctx)
>  {
> @@ -298,6 +395,10 @@ static int cr_read_task(struct cr_ctx *ctx)
>               goto out;
>       ret = cr_read_cpu(ctx);
>       cr_debug("cpu: ret %d\n", ret);
> +     if (ret < 0)
> +             goto out;
> +     ret = cr_read_namespaces(ctx);
> +     cr_debug("ns: ret %d\n", ret);
> 
>   out:
>       return ret;
> diff --git a/include/linux/checkpoint.h b/include/linux/checkpoint.h
> index 2e99c74..cb62716 100644
> --- a/include/linux/checkpoint.h
> +++ b/include/linux/checkpoint.h
> @@ -75,6 +75,7 @@ extern void cr_ctx_put(struct cr_ctx *ctx);
>  enum {
>       CR_OBJ_FILE = 1,
>       CR_OBJ_INODE,
> +     CR_OBJ_UTSNS,
>       CR_OBJ_MAX
>  };
> 
> diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
> index 3addb48..6f29a72 100644
> --- a/include/linux/checkpoint_hdr.h
> +++ b/include/linux/checkpoint_hdr.h
> @@ -48,6 +48,8 @@ enum {
>       CR_HDR_TASK,
>       CR_HDR_THREAD,
>       CR_HDR_CPU,
> +     CR_HDR_NS,
> +     CR_HDR_UTSNS,
> 
>       CR_HDR_MM = 201,
>       CR_HDR_VMA,
> @@ -177,4 +179,13 @@ struct cr_hdr_fd_pipe {
>       __s32 nr_bufs;
>  } __attribute__((aligned(8)));
> 
> +struct cr_hdr_namespaces {
> +     __u32 uts_ref;
> +};
> +
> +struct cr_hdr_utsns {
> +     __u32 nodename_len;
> +     __u32 domainname_len;
> +};
> +
>  #endif /* _CHECKPOINT_CKPT_HDR_H_ */
> -- 
> 1.5.6.3
> 
> _______________________________________________
> Containers mailing list
> [email protected]
> https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Containers mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/containers

_______________________________________________
Devel mailing list
[email protected]
https://openvz.org/mailman/listinfo/devel

Reply via email to