[Devel] [CRIU] [PATCH] spfs, files-reg: symbolic links support
Pavel Tikhomirov
ptikhomirov at virtuozzo.com
Mon Dec 23 17:23:44 MSK 2019
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 at virtuozzo.com>
> Signed-off-by: Alexander Mikhalitsyn <alexander at 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.
--
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.
More information about the Devel
mailing list