files: remove link_remaps when everything has been restored

Now we remove a link_remap file when all its files are restored.
It is wrong, because something else may fail and we will not be
able to repeat an attempt to restore processes again.

travis-ci: success for files: remove link_remaps when everything has been restored
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
Signed-off-by: Pavel Emelyanov <xemul@virtuozzo.com>

criu/cr-restore.c

@@ -1631,7 +1631,7 @@ static int restore_root_task(struct pstree_item *init)
 {
 	enum trace_flags flag = TRACE_ALL;
 	int ret, fd, mnt_ns_fd = -1;
-	int clean_remaps = 1, root_seized = 0;
+	int root_seized = 0;
 	struct pstree_item *item;
 
 	ret = run_scripts(ACT_PRE_RESTORE);
@@ -1773,16 +1773,6 @@ static int restore_root_task(struct pstree_item *init)
 	if (ret < 0)
 		goto out_kill;
 
-	/*
-	 * There is no need to call try_clean_remaps() after this point,
-	 * as restore went OK and all ghosts were removed by the openers.
-	 */
-	if (depopulate_roots_yard(mnt_ns_fd, false))
-		goto out_kill;
-
-	clean_remaps = 0;
-	close_safe(&mnt_ns_fd);
-
 	ret = stop_usernsd();
 	if (ret < 0)
 		goto out_kill;
@@ -1806,6 +1796,15 @@ static int restore_root_task(struct pstree_item *init)
 		goto out_kill;
 	}
 
+	/*
+	 * There is no need to call try_clean_remaps() after this point,
+	 * as restore went OK and all ghosts were removed by the openers.
+	 */
+	if (depopulate_roots_yard(mnt_ns_fd, false))
+		goto out_kill;
+
+	close_safe(&mnt_ns_fd);
+
 	if (write_restored_pid())
 		goto out_kill;
 
@@ -1888,8 +1887,7 @@ out_kill:
 
 out:
 	fini_cgroup();
-	if (clean_remaps)
-		depopulate_roots_yard(mnt_ns_fd, true);
+	depopulate_roots_yard(mnt_ns_fd, true);
 	stop_usernsd();
 	__restore_switch_stage(CR_STATE_FAIL);
 	pr_err("Restoring FAILED.\n");

criu/files-reg.c

@@ -238,12 +238,10 @@ static inline void ghost_path(char *path, int plen,
 	snprintf(path, plen, "%s.cr.%x.ghost", rfi->path, rfe->remap_id);
 }
 
-static int open_remap_ghost(struct reg_file_info *rfi,
+static int collect_remap_ghost(struct reg_file_info *rfi,
 		RemapFilePathEntry *rfe)
 {
 	struct ghost_file *gf;
-	GhostFileEntry *gfe = NULL;
-	struct cr_img *img;
 
 	list_for_each_entry(gf, &ghost_files, list)
 		if (gf->id == rfe->remap_id)
@@ -261,9 +259,28 @@ static int open_remap_ghost(struct reg_file_info *rfi,
 	if (!gf)
 		return -1;
 
-	gf->remap.rpath = xmalloc(PATH_MAX);
+	gf->remap.rpath = shmalloc(PATH_MAX);
 	if (!gf->remap.rpath)
-		goto err;
+		return -1;
+	gf->remap.rpath[0] = 0;
+	gf->id = rfe->remap_id;
+	list_add_tail(&gf->list, &ghost_files);
+
+gf_found:
+	rfi->is_dir = gf->remap.is_dir;
+	rfi->remap = &gf->remap;
+	return 0;
+}
+
+static int open_remap_ghost(struct reg_file_info *rfi,
+					RemapFilePathEntry *rfe)
+{
+	struct ghost_file *gf = container_of(rfi->remap, struct ghost_file, remap);
+	GhostFileEntry *gfe = NULL;
+	struct cr_img *img;
+
+	if (rfi->remap->rpath[0])
+		return 0;
 
 	img = open_image(CR_FD_GHOST_FILE, O_RSTR, rfe->remap_id);
 	if (!img)
@@ -292,15 +309,9 @@ static int open_remap_ghost(struct reg_file_info *rfi,
 	ghost_file_entry__free_unpacked(gfe, NULL);
 	close_image(img);
 
-	gf->id = rfe->remap_id;
-	gf->remap.users = 0;
 	gf->remap.is_dir = S_ISDIR(gfe->mode);
 	gf->remap.uid = gfe->uid;
 	gf->remap.gid = gfe->gid;
-	list_add_tail(&gf->list, &ghost_files);
-gf_found:
-	rfi->is_dir = gf->remap.is_dir;
-	rfi->remap = &gf->remap;
 	return 0;
 
 close_ifd:
@@ -313,14 +324,12 @@ err:
 	return -1;
 }
 
-static int open_remap_linked(struct reg_file_info *rfi,
+static int collect_remap_linked(struct reg_file_info *rfi,
 		RemapFilePathEntry *rfe)
 {
 	struct file_remap *rm;
 	struct file_desc *rdesc;
 	struct reg_file_info *rrfi;
-	uid_t uid = -1;
-	gid_t gid = -1;
 
 	rdesc = find_file_desc_raw(FD_TYPES__REG, rfe->remap_id);
 	if (!rdesc) {
@@ -335,28 +344,32 @@ static int open_remap_linked(struct reg_file_info *rfi,
 	rrfi = container_of(rdesc, struct reg_file_info, d);
 	pr_info("Remapped %s -> %s\n", rfi->path, rrfi->path);
 
+	rm->rpath = rrfi->path;
+	rm->is_dir = false;
+	rm->uid = -1;
+	rm->gid = -1;
+	rm->rmnt_id = rfi->rfe->mnt_id;
+	rfi->remap = rm;
+	return 0;
+}
+
+static int open_remap_linked(struct reg_file_info *rfi,
+		RemapFilePathEntry *rfe)
+{
 	if (root_ns_mask & CLONE_NEWUSER) {
 		int rfd;
 		struct stat st;
 
 		rfd = mntns_get_root_by_mnt_id(rfi->rfe->mnt_id);
-		if (fstatat(rfd, rrfi->path, &st, AT_SYMLINK_NOFOLLOW)) {
-			pr_perror("Can't get owner of link remap %s", rrfi->path);
-			xfree(rm);
+		if (fstatat(rfd, rfi->remap->rpath, &st, AT_SYMLINK_NOFOLLOW)) {
+			pr_perror("Can't get owner of link remap %s", rfi->remap->rpath);
 			return -1;
 		}
 
-		uid = st.st_uid;
-		gid = st.st_gid;
+		rfi->remap->uid = st.st_uid;
+		rfi->remap->gid = st.st_gid;
 	}
 
-	rm->rpath = rrfi->path;
-	rm->users = 0;
-	rm->is_dir = false;
-	rm->uid = uid;
-	rm->gid = gid;
-	rm->rmnt_id = rfi->rfe->mnt_id;
-	rfi->remap = rm;
 	return 0;
 }
 
@@ -419,6 +432,14 @@ static int collect_one_remap(void *obj, ProtobufCMessage *msg, struct cr_img *i)
 
 	ri->rfi = container_of(fdesc, struct reg_file_info, d);
 
+	if (rfe->remap_type == REMAP_TYPE__GHOST) {
+		if (collect_remap_ghost(ri->rfi, ri->rfe))
+			return -1;
+	} else if (rfe->remap_type == REMAP_TYPE__LINKED) {
+		if (collect_remap_linked(ri->rfi, ri->rfe))
+			return -1;
+	}
+
 	list_add_tail(&ri->list, &remaps);
 
 	return 0;
@@ -491,73 +512,57 @@ int prepare_remaps(void)
 	return ret;
 }
 
-static void try_clean_ghost(struct remap_info *ri)
+static int clean_linked_remap(struct remap_info *ri)
 {
 	char path[PATH_MAX];
-	int mnt_id, ret;
+	int mnt_id, ret, rmntns_root;
+	struct file_remap *remap = ri->rfi->remap;
+
+	if (remap->rpath[0] == 0)
+		return 0;
 
 	mnt_id = ri->rfi->rfe->mnt_id; /* rirfirfe %) */
 	ret = rst_get_mnt_root(mnt_id, path, sizeof(path));
 	if (ret < 0)
-		return;
+		return -1;
 	if (ret >= sizeof(path) - 1) {
 		pr_err("The path buffer is too small\n");
-		return;
-	}
-	if (path[ret] != '/') {
-		path[ret++] = '/';
-		path[ret] = 0;
-	}
-
-	if (ri->rfi->remap == NULL)
-		return;
-	if (!ri->rfi->is_dir) {
-		ghost_path(path + ret, sizeof(path) - ret, ri->rfi, ri->rfe);
-		if (!unlink(path)) {
-			pr_info(" `- X [%s] ghost\n", path);
-			return;
-		}
-	} else {
-		strncpy(path + ret, ri->rfi->path, sizeof(path) - ret);
-		if (!rmdir(path)) {
-			pr_info(" `- Xd [%s] ghost\n", path);
-			return;
-		}
+		return -1;
 	}
 
-	pr_perror(" `- XFail [%s] ghost", path);
-}
-
-static int clean_one_remap(struct file_remap *remap)
-{
-	int rmntns_root, ret = 0;
-
-	rmntns_root = mntns_get_root_by_mnt_id(remap->rmnt_id);
-	if (rmntns_root < 0)
+	rmntns_root = open(path, O_RDONLY);
+	if (rmntns_root < 0) {
+		pr_perror("Unbale to open %s", path);
 		return -1;
+	}
 
 	pr_info("Unlink remap %s\n", remap->rpath);
 
 	ret = unlinkat(rmntns_root, remap->rpath, remap->is_dir ? AT_REMOVEDIR : 0);
 	if (ret < 0) {
-		pr_perror("Couldn't unlink remap %s", remap->rpath);
+		pr_perror("Couldn't unlink remap %d %s", rmntns_root, remap->rpath);
 		return -1;
 	}
+	remap->rpath[0] = 0;
 
 	return 0;
 }
 
-void try_clean_remaps()
+int try_clean_remaps(bool only_ghosts)
 {
 	struct remap_info *ri;
+	int ret = 0;
 
-	if (list_empty(&remaps))
-		return;
-
-	list_for_each_entry(ri, &remaps, list)
+	list_for_each_entry(ri, &remaps, list) {
 		if (ri->rfe->remap_type == REMAP_TYPE__GHOST)
-			try_clean_ghost(ri);
+			ret |= clean_linked_remap(ri);
+		else if (only_ghosts)
+			continue;
+		else if (ri->rfe->remap_type == REMAP_TYPE__LINKED)
+			ret |= clean_linked_remap(ri);
+	}
 
+	return ret;
 }
 
 static struct collect_image_info remap_cinfo = {
@@ -626,14 +631,6 @@ static int dump_ghost_file(int _fd, u32 id, const struct stat *st, dev_t phys_de
 	return 0;
 }
 
-void remap_put(struct file_remap *remap)
-{
-	mutex_lock(ghost_file_mutex);
-	if (--remap->users == 0)
-		clean_one_remap(remap);
-	mutex_unlock(ghost_file_mutex);
-}
-
 struct file_remap *lookup_ghost_remap(u32 dev, u32 ino)
 {
 	struct ghost_file *gf;
@@ -641,7 +638,6 @@ struct file_remap *lookup_ghost_remap(u32 dev, u32 ino)
 	mutex_lock(ghost_file_mutex);
 	list_for_each_entry(gf, &ghost_files, list) {
 		if (gf->ino == ino && (gf->dev == dev)) {
-			gf->remap.users++;
 			mutex_unlock(ghost_file_mutex);
 			return &gf->remap;
 		}
@@ -1542,10 +1538,6 @@ ext:
 			rm_parent_dirs(mntns_root, rfi->path, level);
 		}
 
-		BUG_ON(!rfi->remap->users);
-		if (--rfi->remap->users == 0)
-			clean_one_remap(rfi->remap);
-
 		mutex_unlock(ghost_file_mutex);
 	}
 	if (orig_path)
@@ -1659,25 +1651,9 @@ int collect_filemap(struct vma_area *vma)
 	return 0;
 }
 
-static void remap_get(struct file_desc *fdesc, char typ)
-{
-	struct reg_file_info *rfi;
-
-	rfi = container_of(fdesc, struct reg_file_info, d);
-	if (rfi->remap) {
-		pr_debug("One more remap user (%c) for %s\n",
-				typ, rfi->remap->rpath);
-		/* No lock, we're still sngle-process here */
-		rfi->remap->users++;
-	}
-}
-
 static void collect_reg_fd(struct file_desc *fdesc,
 		struct fdinfo_list_entry *fle, struct rst_info *ri)
 {
-	if (list_empty(&fdesc->fd_info_head))
-		remap_get(fdesc, 'f');
-
 	collect_gen_fd(fle, ri);
 }
 
@@ -1719,7 +1695,6 @@ struct file_desc *try_collect_special_file(u32 id, int optional)
 		return NULL;
 	}
 
-	remap_get(fdesc, 's');
 	return fdesc;
 }
 

criu/fsnotify.c

@@ -605,9 +605,6 @@ static int restore_one_inotify(int inotify_fd, struct fsnotify_mark_info *info)
 	}
 
 err:
-	if (info->remap)
-		remap_put(info->remap);
-
 	close_safe(&target);
 	return ret;
 }
@@ -672,9 +669,6 @@ static int restore_one_fanotify(int fd, struct fsnotify_mark_info *mark)
 		}
 	}
 
-	if (mark->remap)
-		remap_put(mark->remap);
-
 err:
 	close_safe(&target);
 	return ret;

criu/include/files-reg.h

@@ -13,7 +13,6 @@ struct file_remap {
 	char *rpath;
 	bool is_dir;
 	int  rmnt_id;
-	unsigned int users;
 	uid_t uid;
 	gid_t gid;
 };
@@ -40,7 +39,6 @@ extern int do_open_reg_noseek_flags(int ns_root_fd, struct reg_file_info *rfi, v
 extern int dump_one_reg_file(int lfd, u32 id, const struct fd_parms *p);
 
 extern struct file_remap *lookup_ghost_remap(u32 dev, u32 ino);
-extern void remap_put(struct file_remap *remap);
 
 extern struct file_desc *try_collect_special_file(u32 id, int optional);
 #define collect_special_file(id)	try_collect_special_file(id, 0)
@@ -51,7 +49,7 @@ extern int collect_remaps_and_regfiles(void);
 extern void delete_link_remaps(void);
 extern void free_link_remaps(void);
 extern int prepare_remaps(void);
-extern void try_clean_remaps(void);
+extern int try_clean_remaps(bool only_ghosts);
 
 extern int strip_deleted(struct fd_link *link);
 

criu/mount.c

@@ -2916,15 +2916,14 @@ static int __depopulate_roots_yard(void)
 	return ret;
 }
 
-int depopulate_roots_yard(int mntns_fd, bool clean_remaps)
+int depopulate_roots_yard(int mntns_fd, bool only_ghosts)
 {
 	int ret = 0, old_cwd = -1, old_ns = -1;
 
 	if (mntns_fd < 0) {
-		if (clean_remaps)
-			try_clean_remaps();
+		ret |= try_clean_remaps(only_ghosts);
 		cleanup_mnt_ns();
-		return 0;
+		return ret;
 	}
 
 	pr_info("Switching to new ns to clean ghosts\n");
@@ -2948,8 +2947,8 @@ int depopulate_roots_yard(int mntns_fd, bool clean_remaps)
 		return -1;
 	}
 
-	if (clean_remaps)
-		try_clean_remaps();
+	if (try_clean_remaps(only_ghosts))
+		ret = -1;
 
 	if (__depopulate_roots_yard())
 		ret = -1;