[CRIU] [PATCH 2/2] introduce --skip-mnt cli option

[Oleg Nesterov]

On 04/02, Pavel Emelyanov wrote:
>
> On 04/01/2015 04:07 PM, Oleg Nesterov wrote:
>
> > Perhaps it should print link->name? Otherwise it is absolutely not clear
> > what this message actually means.
>
> Well yes, adding file name would help but it will still be unclear what
> to do in this case.

Yes... but probably I'll send the patch anyway.

Note that this is possible without --skip-mnt. The target can have an opened
file from another mnt namespace. Say, tty. This is another problem btw.

> I have a general ... food for a thought -- if something fails on dump, what
> should we tell into logs to help user understand what's going on? I admit
> there's no universal answer to this question.

Come on. criu has no bugs. This is impossible.

> >> Such behavior is actually not
> >> good, restore shouldn't fail due to badly created images :)
> >
> > Sure ;) And probably this option can cause such kind of problem. But again,
> > the user should know what he does or do not use this option. This is like
> > --enable-fs, use at your own risk.
>
> Actually if --skip-mnt succeeds the dump and fails the restore, then we
> will have to fix the dump (to fail).

Yes agreed... Just I think that this option is not safe by definition,
unless the user knows what he does.

Even if "restore" succeeds, the task can fail later just because it will
(say) try to open a file on that mountpoint. But yes sure, I agree.

> After explaining why opened files dump fails only two things, one is
> the "usage" string (you mention it) and the comment in mount.c why
> just dropping the mount is OK.

OK. I am sending V2 in reply to V1, you can see the interdiff below.
I also updated the changelog to mention fnmatch().

Thanks!

Oleg.


diff --git a/crtools.c b/crtools.c
index 3e01d33..dd7aaf8 100644
--- a/crtools.c
+++ b/crtools.c
@@ -652,6 +652,7 @@ usage:
 "                        change the root cgroup the controller will be\n"
 "                        installed into. No controller means that root is the\n"
 "                        default for all controllers not specified.\n"
+"  --skip-mnt PATH       ignore this mountpoint when dumping the mount namespace.\n"
 "\n"
 "* Logging:\n"
 "  -o|--log-file FILE    log file name\n"
diff --git a/proc_parse.c b/proc_parse.c
index 7907b16..31ec5fc 100644
--- a/proc_parse.c
+++ b/proc_parse.c
@@ -1035,6 +1035,11 @@ struct mount_info *parse_mountinfo(pid_t pid, struct ns_id *nsid, bool for_dump)
 			goto end;
 		}
 
+		/*
+		 * Drop this mountpoint early, so that lookup_mnt_id/etc will
+		 * fail loudly at "dump" stage if an opened file or another mnt
+		 * depends on this one.
+		 */
 		if (for_dump && should_skip_mount(new->mountpoint + 1)) {
 			pr_info("\tskip %s @ %s\n", fsname,  new->mountpoint);
 			mnt_entry_free(new);