[CRIU] spfs, files-reg: symbolic links support

Submitted by Alexander Mikhalitsyn on Dec. 20, 2019, 8:18 a.m.

Details

Message ID 20191220081810.24512-1-alexander.mikhalitsyn@virtuozzo.com
State New
Series "spfs, files-reg: symbolic links support"
Headers show

Commit Message

Alexander Mikhalitsyn Dec. 20, 2019, 8:18 a.m.
This patch adds support for symbolic links that located on NFS filesystem,
also it introduces support for ghost symbolic links located on non-NFS filesystems.

Caution: ghost symbolic links on NFS are not supported!

Fixes https://jira.sw.ru/browse/PSBM-99969

Signed-off-by: Alexander Mikhalitsyn <alexander.mikhalitsyn@virtuozzo.com>
Signed-off-by: Alexander Mikhalitsyn <alexander@mihalicyn.com>

---
 criu/files-reg.c | 148 ++++++++++++++++++++++++++++++++++++++++++++-----------
 criu/spfs.c      |   2 +-
 2 files changed, 119 insertions(+), 31 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/files-reg.c b/criu/files-reg.c
index 0d8e2e3..b7ba2db 100644
--- a/criu/files-reg.c
+++ b/criu/files-reg.c
@@ -315,19 +315,64 @@  static int mkreg_ghost(char *path, GhostFileEntry *gfe, struct cr_img *img)
 	return ret;
 }
 
+static int mklnk_ghost(char *path, GhostFileEntry *gfe, struct cr_img *img)
+{
+	char pathbuf[PATH_MAX] = "CRIU_GHOST_SYMLINK_PLACEHOLDER";
+	int ret;
+
+	if (!gfe->has_size) {
+		pr_err("Corrupted ghost (symlink) image -> no size\n");
+		return -1;
+	}
+
+	/*
+	 * gfe->size could be zero, but in real we can't create
+	 * symlink with empty target.
+	 * If gfe->size = 0 therefore we works with ghost symlink
+	 * that was dumped without content => this symlink lives on spfs.
+	 * This is not a problem to make it with fake target
+	 * because later spfs will be remounted to NFS and symlink will have
+	 * original target.
+	 */
+	if (gfe->size) {
+		ret = read(img_raw_fd(img), pathbuf, gfe->size);
+		if (ret < 0) {
+			pr_perror("Can't read from image\n");
+			return -1;
+		}
+
+		pathbuf[ret] = 0;
+	}
+
+	ret = symlink(pathbuf, path);
+	if (ret < 0) {
+		pr_perror("Could not create ghost symlink %s\n", path);
+		return -1;
+	}
+
+	return 0;
+}
+
 static int ghost_apply_metadata(const char *path, GhostFileEntry *gfe)
 {
 	struct timeval tv[2];
 	int ret = -1;
 
-	if (chown(path, gfe->uid, gfe->gid) < 0) {
-		pr_perror("Can't reset user/group on ghost %s", path);
-		goto err;
-	}
+	if (S_ISLNK(gfe->mode)) {
+		if (lchown(path, gfe->uid, gfe->gid) < 0) {
+			pr_perror("Can't reset user/group on ghost %s", path);
+			goto err;
+		}
+	} else {
+		if (chown(path, gfe->uid, gfe->gid) < 0) {
+			pr_perror("Can't reset user/group on ghost %s", path);
+			goto err;
+		}
 
-	if (chmod(path, gfe->mode)) {
-		pr_perror("Can't set perms %o on ghost %s", gfe->mode, path);
-		goto err;
+		if (chmod(path, gfe->mode)) {
+			pr_perror("Can't set perms %o on ghost %s", gfe->mode, path);
+			goto err;
+		}
 	}
 
 	if (gfe->atim) {
@@ -365,6 +410,9 @@  again:
 	} else if (S_ISDIR(gfe->mode)) {
 		if ((ret = mkdirpat(AT_FDCWD, path, gfe->mode)) < 0)
 			msg = "Can't make ghost dir";
+	} else if (S_ISLNK(gfe->mode)) {
+		if ((ret = mklnk_ghost(path, gfe, img)) < 0)
+			msg = "Can't create ghost symlink";
 	} else {
 		if ((ret = mkreg_ghost(path, gfe, img)) < 0)
 			msg = "Can't create ghost regfile";
@@ -1031,6 +1079,7 @@  static int dump_ghost_file(int _fd, u32 id, const struct stat *st,
 	struct cr_img *img;
 	GhostFileEntry gfe = GHOST_FILE_ENTRY__INIT;
 	Timeval atim = TIMEVAL__INIT, mtim = TIMEVAL__INIT;
+	int ret = -1;
 
 	pr_info("Dumping ghost file contents (id %#x)\n", id);
 
@@ -1064,35 +1113,74 @@  static int dump_ghost_file(int _fd, u32 id, const struct stat *st,
 		gfe.size = st->st_size;
 	}
 
-	if (pb_write_one(img, &gfe, PB_GHOST_FILE))
-		return -1;
-
-	if (S_ISREG(st->st_mode) && dump_content) {
-		int fd, ret;
-		char lpath[PSFDS];
-
+	if (S_ISLNK(st->st_mode)) {
+		gfe.has_size = true;
 		/*
-		 * Reopen file locally since it may have no read
-		 * permissions when drained
+		 * We set gfe.size to st_size only if we need to dump
+		 * symlink content, otherwise we set size to 0.
+		 * It will be important on restore in mklnk_ghost function.
 		 */
-		sprintf(lpath, "/proc/self/fd/%d", _fd);
-		fd = open(lpath, O_RDONLY);
-		if (fd < 0) {
-			pr_perror("Can't open ghost original file");
-			return -1;
-		}
+		gfe.size = dump_content ? st->st_size : 0;
+	}
 
-		if (gfe.chunks)
-			ret = copy_file_to_chunks(fd, img, st->st_size);
-		else
-			ret = copy_file(fd, img_raw_fd(img), st->st_size);
-		close(fd);
-		if (ret)
-			return -1;
+	if ((ret = pb_write_one(img, &gfe, PB_GHOST_FILE)))
+		goto exit_close_image;
+
+	if (dump_content) {
+		if (S_ISREG(st->st_mode)) {
+			int fd, flags = O_RDONLY;
+			char lpath[PSFDS];
+
+			/*
+			 * Reopen file locally since it may have no read
+			 * permissions when drained
+			 */
+			sprintf(lpath, "/proc/self/fd/%d", _fd);
+			fd = open(lpath, flags);
+			if (fd < 0) {
+				pr_perror("Can't open ghost original file");
+				goto exit_close_image;
+			}
+
+			if (gfe.chunks)
+				ret = copy_file_to_chunks(fd, img, st->st_size);
+			else
+				ret = copy_file(fd, img_raw_fd(img), st->st_size);
+
+			close(fd);
+		} else if (S_ISLNK(st->st_mode)) {
+			char pathbuf[PATH_MAX];
+
+			/*
+			 * We assume that _fd opened with O_PATH | O_NOFOLLOW
+			 * flags because S_ISLNK(st->st_mode). With current kernel version,
+			 * it's looks like correct assumption in any case.
+			 */
+			ret = readlinkat(_fd, "", pathbuf, sizeof(pathbuf));
+			if (ret < 0) {
+				pr_perror("Can't readlinkat");
+				goto exit_close_image;
+			}
+
+			pathbuf[ret] = 0;
+
+			if (ret != st->st_size) {
+				pr_err("Buffer for readlinkat is too small: ret %u, st_size %u, buf %u %s", (unsigned)ret, (unsigned)st->st_size, (unsigned)PATH_MAX, pathbuf);
+				goto exit_close_image;
+			}
+
+			ret = write(img_raw_fd(img), pathbuf, st->st_size);
+			if (ret < 0) {
+				pr_perror("Can't write link data to image\n");
+				goto exit_close_image;
+			}
+			ret = 0;
+		}
 	}
 
+exit_close_image:
 	close_image(img);
-	return 0;
+	return ret ? -1 : 0;
 }
 
 struct file_remap *lookup_ghost_remap(u32 dev, u32 ino)
diff --git a/criu/spfs.c b/criu/spfs.c
index 730de13..20182c2 100644
--- a/criu/spfs.c
+++ b/criu/spfs.c
@@ -78,7 +78,7 @@  static int spfs_send_request(int sock, void *req, size_t len)
 
 int spfs_remap_path(const char *path, const char *link_remap)
 {
-	if (setxattr(path, "security.spfs.link_remap", link_remap, strlen(link_remap) + 1,  XATTR_CREATE)) {
+	if (lsetxattr(path, "security.spfs.link_remap", link_remap, strlen(link_remap) + 1,  XATTR_CREATE)) {
 		pr_perror("failed to set xattr security.spfs.link_remap with value %s for file %s", link_remap, path);
 		return -1;
 	}

Comments

Pavel Tikhomirov Dec. 23, 2019, 2:23 p.m.
On 12/20/19 11:18 AM, Alexander Mikhalitsyn wrote:
> This patch adds support for symbolic links that located on NFS filesystem,
> also it introduces support for ghost symbolic links located on non-NFS filesystems.
> 
> Caution: ghost symbolic links on NFS are not supported!
> 
> Fixes https://jira.sw.ru/browse/PSBM-99969
> 
> Signed-off-by: Alexander Mikhalitsyn <alexander.mikhalitsyn@virtuozzo.com>
> Signed-off-by: Alexander Mikhalitsyn <alexander@mihalicyn.com>
> 
> ---
>   criu/files-reg.c | 148 ++++++++++++++++++++++++++++++++++++++++++++-----------
>   criu/spfs.c      |   2 +-
>   2 files changed, 119 insertions(+), 31 deletions(-)
> 
> diff --git a/criu/files-reg.c b/criu/files-reg.c
> index 0d8e2e3..b7ba2db 100644
> --- a/criu/files-reg.c
> +++ b/criu/files-reg.c
> @@ -315,19 +315,64 @@ static int mkreg_ghost(char *path, GhostFileEntry *gfe, struct cr_img *img)
>   	return ret;
>   }
>   
> +static int mklnk_ghost(char *path, GhostFileEntry *gfe, struct cr_img *img)
> +{
> +	char pathbuf[PATH_MAX] = "CRIU_GHOST_SYMLINK_PLACEHOLDER";
> +	int ret;
> +
> +	if (!gfe->has_size) {
> +		pr_err("Corrupted ghost (symlink) image -> no size\n");
> +		return -1;
> +	}
> +
> +	/*
> +	 * gfe->size could be zero, but in real we can't create
> +	 * symlink with empty target.
> +	 * If gfe->size = 0 therefore we works with ghost symlink
> +	 * that was dumped without content => this symlink lives on spfs.
> +	 * This is not a problem to make it with fake target
> +	 * because later spfs will be remounted to NFS and symlink will have
> +	 * original target.
> +	 */
> +	if (gfe->size) {
> +		ret = read(img_raw_fd(img), pathbuf, gfe->size);
> +		if (ret < 0) {
> +			pr_perror("Can't read from image\n");
> +			return -1;
> +		}
> +
> +		pathbuf[ret] = 0;
> +	}
> +
> +	ret = symlink(pathbuf, path);
> +	if (ret < 0) {
> +		pr_perror("Could not create ghost symlink %s\n", path);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>   static int ghost_apply_metadata(const char *path, GhostFileEntry *gfe)
>   {
>   	struct timeval tv[2];
>   	int ret = -1;
>   
> -	if (chown(path, gfe->uid, gfe->gid) < 0) {
> -		pr_perror("Can't reset user/group on ghost %s", path);
> -		goto err;
> -	}
> +	if (S_ISLNK(gfe->mode)) {
> +		if (lchown(path, gfe->uid, gfe->gid) < 0) {
> +			pr_perror("Can't reset user/group on ghost %s", path);
> +			goto err;
> +		}

Maybe we need some sort of open(O_PATH|O_NOFOLLOW) + fchmod here also 
same as for regular files?

> +	} else {
> +		if (chown(path, gfe->uid, gfe->gid) < 0) {
> +			pr_perror("Can't reset user/group on ghost %s", path);
> +			goto err;
> +		}
>   
> -	if (chmod(path, gfe->mode)) {
> -		pr_perror("Can't set perms %o on ghost %s", gfe->mode, path);
> -		goto err;
> +		if (chmod(path, gfe->mode)) {
> +			pr_perror("Can't set perms %o on ghost %s", gfe->mode, path);
> +			goto err;
> +		}
>   	}
>   
>   	if (gfe->atim) {
> @@ -365,6 +410,9 @@ again:
>   	} else if (S_ISDIR(gfe->mode)) {
>   		if ((ret = mkdirpat(AT_FDCWD, path, gfe->mode)) < 0)
>   			msg = "Can't make ghost dir";
> +	} else if (S_ISLNK(gfe->mode)) {
> +		if ((ret = mklnk_ghost(path, gfe, img)) < 0)
> +			msg = "Can't create ghost symlink";
>   	} else {
>   		if ((ret = mkreg_ghost(path, gfe, img)) < 0)
>   			msg = "Can't create ghost regfile";
> @@ -1031,6 +1079,7 @@ static int dump_ghost_file(int _fd, u32 id, const struct stat *st,
>   	struct cr_img *img;
>   	GhostFileEntry gfe = GHOST_FILE_ENTRY__INIT;
>   	Timeval atim = TIMEVAL__INIT, mtim = TIMEVAL__INIT;
> +	int ret = -1;
>   
>   	pr_info("Dumping ghost file contents (id %#x)\n", id);
>   
> @@ -1064,35 +1113,74 @@ static int dump_ghost_file(int _fd, u32 id, const struct stat *st,
>   		gfe.size = st->st_size;
>   	}
>   
> -	if (pb_write_one(img, &gfe, PB_GHOST_FILE))
> -		return -1;
> -
> -	if (S_ISREG(st->st_mode) && dump_content) {
> -		int fd, ret;
> -		char lpath[PSFDS];
> -
> +	if (S_ISLNK(st->st_mode)) {

Same as I told in personal, may be we can use some field in protobuf 
image (e.g. gfe->symlink) instead of creating another raw image with 
just a single path in it.

> +		gfe.has_size = true;
>   		/*
> -		 * Reopen file locally since it may have no read
> -		 * permissions when drained
> +		 * We set gfe.size to st_size only if we need to dump
> +		 * symlink content, otherwise we set size to 0.
> +		 * It will be important on restore in mklnk_ghost function.
>   		 */
> -		sprintf(lpath, "/proc/self/fd/%d", _fd);
> -		fd = open(lpath, O_RDONLY);
> -		if (fd < 0) {
> -			pr_perror("Can't open ghost original file");
> -			return -1;
> -		}
> +		gfe.size = dump_content ? st->st_size : 0;
> +	}
>   
> -		if (gfe.chunks)
> -			ret = copy_file_to_chunks(fd, img, st->st_size);
> -		else
> -			ret = copy_file(fd, img_raw_fd(img), st->st_size);
> -		close(fd);
> -		if (ret)
> -			return -1;
> +	if ((ret = pb_write_one(img, &gfe, PB_GHOST_FILE)))
> +		goto exit_close_image;
> +
> +	if (dump_content) {
> +		if (S_ISREG(st->st_mode)) {
> +			int fd, flags = O_RDONLY;
> +			char lpath[PSFDS];
> +
> +			/*
> +			 * Reopen file locally since it may have no read
> +			 * permissions when drained
> +			 */
> +			sprintf(lpath, "/proc/self/fd/%d", _fd);
> +			fd = open(lpath, flags);
> +			if (fd < 0) {
> +				pr_perror("Can't open ghost original file");
> +				goto exit_close_image;
> +			}
> +
> +			if (gfe.chunks)
> +				ret = copy_file_to_chunks(fd, img, st->st_size);
> +			else
> +				ret = copy_file(fd, img_raw_fd(img), st->st_size);
> +
> +			close(fd);
> +		} else if (S_ISLNK(st->st_mode)) {
> +			char pathbuf[PATH_MAX];
> +
> +			/*
> +			 * We assume that _fd opened with O_PATH | O_NOFOLLOW
> +			 * flags because S_ISLNK(st->st_mode). With current kernel version,
> +			 * it's looks like correct assumption in any case.
> +			 */
> +			ret = readlinkat(_fd, "", pathbuf, sizeof(pathbuf));
> +			if (ret < 0) {
> +				pr_perror("Can't readlinkat");
> +				goto exit_close_image;
> +			}
> +
> +			pathbuf[ret] = 0;
> +
> +			if (ret != st->st_size) {
> +				pr_err("Buffer for readlinkat is too small: ret %u, st_size %u, buf %u %s", (unsigned)ret, (unsigned)st->st_size, (unsigned)PATH_MAX, pathbuf);
> +				goto exit_close_image;
> +			}
> +
> +			ret = write(img_raw_fd(img), pathbuf, st->st_size);
> +			if (ret < 0) {
> +				pr_perror("Can't write link data to image\n");
> +				goto exit_close_image;
> +			}
> +			ret = 0;
> +		}
>   	}
>   
> +exit_close_image:
>   	close_image(img);
> -	return 0;
> +	return ret ? -1 : 0;
>   }
>   
>   struct file_remap *lookup_ghost_remap(u32 dev, u32 ino)
> diff --git a/criu/spfs.c b/criu/spfs.c
> index 730de13..20182c2 100644
> --- a/criu/spfs.c
> +++ b/criu/spfs.c
> @@ -78,7 +78,7 @@ static int spfs_send_request(int sock, void *req, size_t len)
>   
>   int spfs_remap_path(const char *path, const char *link_remap)
>   {
> -	if (setxattr(path, "security.spfs.link_remap", link_remap, strlen(link_remap) + 1,  XATTR_CREATE)) {
> +	if (lsetxattr(path, "security.spfs.link_remap", link_remap, strlen(link_remap) + 1,  XATTR_CREATE)) {
>   		pr_perror("failed to set xattr security.spfs.link_remap with value %s for file %s", link_remap, path);
>   		return -1;
>   	}
> 

Also as we now know that ghost-files on spfs are not supported please 
add something like:

           if (ost->st_nlink == 0) {
+                 if (spfs_file(parms, nsid))
+                                 return -1;

to check_path_remap() to detect problems earlier.