make cr depend on all namespaces

This should let us get rid of some ifdefed code and reduce chances for bad config combinations. There's really no reason to support it.

This should let us get rid of some ifdefed code and reduce chances for bad config combinations. There's really no reason to support it.

Signed-off-by Serge E. Hallyn <serue-r/[email protected]>

---
 checkpoint/Kconfig     |    5 +++++
 checkpoint/namespace.c |   10 ----------
 2 files changed, 5 insertions(+), 10 deletions(-)

checkpoint/Kconfig +5 -0

index 4a2c845..549668e 100644
@@ -9,6 +9,11 @@ config DEFERQUEUE
 config CHECKPOINT
 	bool "Checkpoint/restart (EXPERIMENTAL)"
 	depends on CHECKPOINT_SUPPORT && EXPERIMENTAL
+	depends on UTS_NS
+	depends on USER_NS
+	depends on PID_NS
+	depends on NET_NS
+	depends on DEVPTS_MULTIPLE_INSTANCES
 	depends on CGROUP_FREEZER
 	select DEFERQUEUE
 	help

checkpoint/namespace.c +0 -10

index 4b3ac5a..6389dbd 100644
@@ -63,7 +63,6 @@ static struct uts_namespace *do_restore_uts_ns(struct ckpt_ctx *ctx)
 	if (IS_ERR(h))
 		return (struct uts_namespace *) h;
 
-#ifdef CONFIG_UTS_NS
 	uts_ns = create_uts_ns();
 	if (!uts_ns) {
 		uts_ns = ERR_PTR(-ENOMEM);

@@ -78,15 +77,6 @@ static struct uts_namespace *do_restore_uts_ns(struct ckpt_ctx *ctx)
 	memcpy(name->machine, h->machine, sizeof(name->machine));
 	memcpy(name->domainname, h->domainname, sizeof(name->domainname));
 	up_read(&uts_sem);
-#else
-	/* complain if image contains multiple namespaces */
-	if (ctx->stats.uts_ns) {
-		uts_ns = ERR_PTR(-EEXIST);
-		goto out;
-	}
-	uts_ns = current->nsproxy->uts_ns;
-	get_uts_ns(uts_ns);
-#endif
 
 	ctx->stats.uts_ns++;
  out:

Quoted 3 lines · Serge E. Hallyn

This should let us get rid of some ifdefed code and reduce chances for bad config combinations. There's really no reason to support it.

I disagree.

You are right that this will reduce the changes of bad config combinations.

However, it will also introduce some restrictions on the kernel config which are unnecessary. Some people may not want to have all namespaces configured.

Note that the namespaces are independent in the sense that we don't need to test all combination of all namespaces - instead, I consider turning on/off one at a time to be safe enough.

(FWIW, is it because you only wanted to show a point that you only remove UTS_NS ifdefs ?)

Quoted 10 lines · Oren Laadan

Quoted 3 lines · Serge E. Hallyn

This should let us get rid of some ifdefed code and reduce chances for bad config combinations. There's really no reason to support it.

I disagree.

You are right that this will reduce the changes of bad config combinations.

Quoted 3 lines

However, it will also introduce some restrictions on the kernel config which are unnecessary. Some people may not want to have all namespaces configured.

Why? The only reason right now to disable namespaces is for kernel size.

Quoted 4 lines

Note that the namespaces are independent in the sense that we don't need to test all combination of all namespaces - instead, I consider turning on/off one at a time to be safe enough.

And do you do that? :) It still gets more complicated bc you have things like sysvipc and posix mq which both can allow ipc_ns.

Quoted 3 lines

(FWIW, is it because you only wanted to show a point that you only remove UTS_NS ifdefs ?)

It was just right there in my face...

Anyway if you don't take this patch then the UTS_NS code I removed should have 'name' put under ifdef to avoid a build warning.

Quoted 11 lines · Serge E. Hallyn

Quoted 10 lines · Oren Laadan

Quoted 3 lines · Serge E. Hallyn

This should let us get rid of some ifdefed code and reduce chances for bad config combinations. There's really no reason to support it.

I disagree.

You are right that this will reduce the changes of bad config combinations.

Quoted 6 lines

Quoted 3 lines

However, it will also introduce some restrictions on the kernel config which are unnecessary. Some people may not want to have all namespaces configured.

Why? The only reason right now to disable namespaces is for kernel size.

Yup.

So we know it works well when all ns are enabled. If there are so few users that disable some ns, then we will rarely hear about breakage anyway.

On the other hand - what about current distributions - do they enable all namespaces by default ? If not (mine didn't), then potential tester will haev yet another barrier to testing since, for instance, net-ns is disabled.

Quoted 8 lines

Quoted 4 lines

Note that the namespaces are independent in the sense that we don't need to test all combination of all namespaces - instead, I consider turning on/off one at a time to be safe enough.

And do you do that? :) It still gets more complicated bc you have things like sysvipc and posix mq which both can allow ipc_ns.

You are 100% correct - we don't, and we should automate it.

I thought you intentionally leave out IPC in that patch... ?

Anyway, ipc is the exception, because it can be disabled as a whole. Can you do that on others ? (e.g. uts, user, etc)

Quoted 4 lines

Quoted 3 lines

(FWIW, is it because you only wanted to show a point that you only remove UTS_NS ifdefs ?)

Quoted 5 lines

It was just right there in my face...

Anyway if you don't take this patch then the UTS_NS code I removed should have 'name' put under ifdef to avoid a build warning.

Ok, will patch away and push to v20-rc2.