[CRIU] [PATCH v3 1/6] files-reg: Account number of unlinked parent components of ghost file
Pavel Emelyanov
xemul at parallels.com
Mon Jan 25 01:54:18 PST 2016
On 01/19/2016 08:07 PM, Kirill Tkhai wrote:
> On 18.01.2016 21:05, Pavel Emelyanov wrote:
>> On 12/29/2015 05:05 PM, Kirill Tkhai wrote:
>>> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
>>> ---
>>> files-reg.c | 69 ++++++++++++++++++++++++++++++++++++++-------
>>> protobuf/ghost-file.proto | 1 +
>>> 2 files changed, 60 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/files-reg.c b/files-reg.c
>>> index 0f89be5..3e09ae0 100644
>>> --- a/files-reg.c
>>> +++ b/files-reg.c
>>> @@ -48,6 +48,7 @@ struct ghost_file {
>>>
>>> u32 dev;
>>> u32 ino;
>>> + u32 nr_up;
>>>
>>> struct file_remap remap;
>>> };
>>> @@ -223,6 +224,7 @@ static int open_remap_ghost(struct reg_file_info *rfi,
>>> gf->dev = gfe->dev;
>>> gf->ino = gfe->ino;
>>> gf->remap.rmnt_id = rfi->rfe->mnt_id;
>>> + gf->nr_up = gfe->has_nr_up ? gfe->nr_up : 0;
>>>
>>> if (S_ISDIR(gfe->mode))
>>> strncpy(gf->remap.rpath, rfi->path, PATH_MAX);
>>> @@ -526,7 +528,50 @@ static struct collect_image_info remap_cinfo = {
>>> .collect = collect_one_remap,
>>> };
>>>
>>> -static int dump_ghost_file(int _fd, u32 id, const struct stat *st, dev_t phys_dev)
>>> +static int count_unlinked_parents(GhostFileEntry *gfe, const char *path, int mntns_root)
>>> +{
>>> + struct stat st;
>>> + char buf[PATH_MAX], *p;
>>> + int count = 0;
>>> +
>>> + strcpy(buf, path);
>>> +
>>> + p = strrchr(buf, '/');
>>> + if (!p) {
>>> + pr_err("No parent for %s", buf);
>>> + count = -1;
>>> + goto out;
>>> + }
>>> +
>>> + while (p > buf) {
>>> + *p = '\0';
>>> + if (fstatat(mntns_root, buf, &st, 0) == 0)
>>
>> This place looks dangerous. Consider we get here after we've fstat()-ed the fd
>> and found out that its n_link is 0. Then we go on and stat() it by path. But
>> the path in question may refer to some other existing file and thus this whole
>> logic becomes flawed.
>
> Below code counts stable parents, i.e. existing directories only. How about this?
>
> ---
> diff --git a/files-reg.c b/files-reg.c
> index ea66af0..8fdab57 100644
> --- a/files-reg.c
> +++ b/files-reg.c
> @@ -48,6 +48,7 @@ struct ghost_file {
>
> u32 dev;
> u32 ino;
> + u32 nr_stable;
>
> struct file_remap remap;
> };
> @@ -223,6 +224,7 @@ static int open_remap_ghost(struct reg_file_info *rfi,
> gf->dev = gfe->dev;
> gf->ino = gfe->ino;
> gf->remap.rmnt_id = rfi->rfe->mnt_id;
> + gf->nr_stable = gfe->has_nr_stable ? gfe->nr_stable : 0;
>
> if (S_ISDIR(gfe->mode))
> strncpy(gf->remap.rpath, rfi->path, PATH_MAX);
> @@ -526,7 +528,47 @@ static struct collect_image_info remap_cinfo = {
> .collect = collect_one_remap,
> };
>
> -static int dump_ghost_file(int _fd, u32 id, const struct stat *st, dev_t phys_dev)
> +static int count_stable_parents(GhostFileEntry *gfe, char *path, int mntns_root)
> +{
> + int ret = 0, count = 0;
> + struct stat64 st;
> + char *p = path;
> +
> + while (1) {
> + p = strchr(p, '/');
> + if (!p)
> + break;
> + *p = '\0';
> +
> + ret = fstatat64(mntns_root, path, &st, AT_SYMLINK_NOFOLLOW);
> + if (ret < 0 && errno != ENOENT) {
> + pr_err("Can't stat on %s\n", path);
> + *p = '/';
> + goto out;
Once you meet the "alive" directory, you should also compare the device
it sits on and mountpoint it's visible via with those on the ghost file
you see via descriptor.
-- Pavel
> + }
> +
> + *p = '/';
> +
> + if (ret < 0)
> + break;
> + if ((st.st_mode & S_IFMT) != S_IFDIR)
> + break;
> + count++;
> + p++;
> + }
> +
> + if (p && count) {
> + gfe->has_nr_stable = true;
> + gfe->nr_stable = count;
> + pr_debug("Ghost file %s has %d stable parent component(s)", path, count);
> + }
> + ret = 0;
> +out:
> + return ret;
> +}
> +
> +static int dump_ghost_file(char *path, int _fd, u32 id, const struct stat *st,
> + dev_t phys_dev, int mntns_root)
> {
> struct cr_img *img;
> GhostFileEntry gfe = GHOST_FILE_ENTRY__INIT;
> @@ -553,6 +595,9 @@ static int dump_ghost_file(int _fd, u32 id, const struct stat *st, dev_t phys_de
> gfe.dev = phys_dev;
> gfe.ino = st->st_ino;
>
> + if (count_stable_parents(&gfe, path, mntns_root) < 0)
> + return -1;
> +
> if (S_ISCHR(st->st_mode) || S_ISBLK(st->st_mode)) {
> gfe.has_rdev = true;
> gfe.rdev = st->st_rdev;
> @@ -616,8 +661,8 @@ struct file_remap *lookup_ghost_remap(u32 dev, u32 ino)
> return NULL;
> }
>
> -static int dump_ghost_remap(char *path, const struct stat *st,
> - int lfd, u32 id, struct ns_id *nsid)
> +static int dump_ghost_remap(char *path, const struct stat *st, int lfd,
> + u32 id, struct ns_id *nsid, int mntns_root)
> {
> struct ghost_file *gf;
> RemapFilePathEntry rpe = REMAP_FILE_PATH_ENTRY__INIT;
> @@ -645,7 +690,7 @@ static int dump_ghost_remap(char *path, const struct stat *st,
> gf->id = ghost_file_ids++;
> list_add_tail(&gf->list, &ghost_files);
>
> - if (dump_ghost_file(lfd, gf->id, st, phys_dev))
> + if (dump_ghost_file(path + 1, lfd, gf->id, st, phys_dev, mntns_root))
> return -1;
>
> dump_entry:
> @@ -934,6 +979,10 @@ static int check_path_remap(struct fd_link *link, const struct fd_parms *parms,
> return 0;
> }
>
> + mntns_root = mntns_get_root_fd(nsid);
> + if (mntns_root < 0)
> + return -1;
> +
> if (ost->st_nlink == 0) {
> /*
> * Unpleasant, but easy case. File is completely invisible
> @@ -942,7 +991,8 @@ static int check_path_remap(struct fd_link *link, const struct fd_parms *parms,
> * also open.
> */
> strip_deleted(link);
> - return dump_ghost_remap(rpath + 1, ost, lfd, id, nsid);
> +
> + return dump_ghost_remap(rpath + 1, ost, lfd, id, nsid, mntns_root);
> }
>
> if (nfs_silly_rename(rpath, parms)) {
> @@ -957,10 +1007,6 @@ static int check_path_remap(struct fd_link *link, const struct fd_parms *parms,
> return dump_linked_remap(rpath + 1, plen - 1, ost, lfd, id, nsid);
> }
>
> - mntns_root = mntns_get_root_fd(nsid);
> - if (mntns_root < 0)
> - return -1;
> -
> ret = fstatat(mntns_root, rpath, &pst, 0);
> if (ret < 0) {
> /*
> diff --git a/protobuf/ghost-file.proto b/protobuf/ghost-file.proto
> index db056db..d74f195 100644
> --- a/protobuf/ghost-file.proto
> +++ b/protobuf/ghost-file.proto
> @@ -11,4 +11,5 @@ message ghost_file_entry {
> optional uint32 rdev = 6 [(criu).dev = true, (criu).odev = true];
> optional timeval atim = 7;
> optional timeval mtim = 8;
> + optional uint32 nr_stable = 9; /* Number of stable parent components */
> }
> .
>
More information about the CRIU
mailing list