[CRIU] [PATCH] Allow dumping of pstore, securityfs, fusectl, debugfs

[Tycho Andersen]

Hi Pavel,

On Tue, Jun 10, 2014 at 04:25:13PM +0400, Pavel Emelyanov wrote:
> On 06/09/2014 09:32 PM, Tycho Andersen wrote:
> > Hi Pavel,
> > 
> > On Mon, Jun 09, 2014 at 08:25:39PM +0400, Pavel Emelyanov wrote:
> >> On 06/09/2014 08:04 PM, Tycho Andersen wrote:
> >>>
> >>> +
> >>>  static struct fstype fstypes[] = {
> >>>  	{
> >>>  		.name = "unsupported",
> >>> @@ -738,6 +750,22 @@ static struct fstype fstypes[] = {
> >>>  	}, {
> >>>  		.name = "btrfs",
> >>>  		.code = FSTYPE__UNSUPPORTED,
> >>> +	}, {
> >>> +		.name = "pstore",
> >>> +		.dump = pstore_dump,
> >>> +		.code = FSTYPE__PSTORE,
> >>
> >> I haven't worked with pstore, thus the question -- what if we
> >> just umount it on one box and mount on another without doing
> >> anything about preserving its contents. Will it affect the container?
> > 
> > pstore is "platform persistent storage", so it should not be migrated
> > with the container. The only real reason it is here is because
> > ubuntu's mountall wants to mount it. The default apparmor rules
> > disallow changes by containers to all of these filesystems.
> > 
> > After some discussion with Serge Hallyn, it sounds like at some point
> > in the future we might allow writes to /sys/fs/fuse, but for now we
> > don't. All this patch really does for us is allow dump to complete
> > successfully so we can start working on restore.
> 
> OK, this is reasonable. I will apply the patch once one more comment
> (below) is resolved. And I'd also appreciate one more patch on top of 
> it that actually checks that the FS we dump is really empty (like it's
> done for binfmt_misc in criu code).

I added an empty check for fuse and pstore in the patch below, the
other two aren't necessarily empty. (In fact, fuse won't be empty
either if the host has some fuse fs mounted, but that's okay for us
for now.) It is essentially the same as binfmt's dump, so I could
refactor things a little bit there if you don't mind a more invasive
patch.

> > +	}, {
> > +		.name = "pstore",
> > +		.dump = pstore_dump,
> 
> This should be not .dump = pstore_dump, but .code = FSTYPE__PSTORE, shouldn't it?

Yes, of course. Apologies for the carelessness.

Tycho




These are mounted by default in ubuntu containers, so criu should know about
them and remount them on restore.

Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
---
 mount.c            | 45 +++++++++++++++++++++++++++++++++++++++++++++
 protobuf/mnt.proto |  4 ++++
 2 files changed, 49 insertions(+)

diff --git a/mount.c b/mount.c
index d770e14..13072b2 100644
--- a/mount.c
+++ b/mount.c
@@ -706,6 +706,37 @@ out:
 	return ret;
 }
 
+
+static int dump_empty_fs(struct mount_info *pm)
+{
+	int fd, ret = -1;
+	struct dirent *de;
+	DIR *fdir = NULL;
+	fd = open_mountpoint(pm);
+
+	if (fd < 0)
+		return -1;
+
+	fdir = fdopendir(fd);
+	if (fdir == NULL) {
+		close(fd);
+		return -1;
+	}
+
+	while ((de = readdir(fdir))) {
+		if (dir_dots(de))
+			continue;
+
+		pr_err("%s isn't empty: %s\n", pm->fstype->name, de->d_name);
+		goto out;
+	}
+
+	ret = 0;
+out:
+	closedir(fdir);
+	return ret;
+}
+
 static struct fstype fstypes[] = {
 	{
 		.name = "unsupported",
@@ -738,6 +769,20 @@ static struct fstype fstypes[] = {
 	}, {
 		.name = "btrfs",
 		.code = FSTYPE__UNSUPPORTED,
+	}, {
+		.name = "pstore",
+		.dump = dump_empty_fs,
+		.code = FSTYPE__PSTORE,
+	}, {
+		.name = "securityfs",
+		.code = FSTYPE__SECURITYFS,
+	}, {
+		.name = "fusectl",
+		.dump = dump_empty_fs,
+		.code = FSTYPE__FUSECTL,
+	}, {
+		.name = "debugfs",
+		.code = FSTYPE__DEBUGFS,
 	}
 };
 
diff --git a/protobuf/mnt.proto b/protobuf/mnt.proto
index ab85de7..297bcbd 100644
--- a/protobuf/mnt.proto
+++ b/protobuf/mnt.proto
@@ -7,6 +7,10 @@ enum fstype {
 	TMPFS			= 5;
 	DEVPTS			= 6;
 	SIMFS			= 7;
+	PSTORE			= 8;
+	SECURITYFS		= 9;
+	FUSECTL			= 10;
+	DEBUGFS			= 11;
 };
 
 message mnt_entry {
-- 
1.9.1