[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 03:07:10 PST 2016
On 01/25/2016 01:28 PM, Kirill Tkhai wrote:
>
>
> On 25.01.2016 12:54, Pavel Emelyanov wrote:
>> 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.
>
> Some clarification about this.
>
> mount /dev/XXX /
> mount /dev/YYY /mnt
>
> open("/mnt/dir/file")
> unlink("/mnt/dir/file")
> rmdir("/mnt/dir")
>
> mount /dev/ZZZ /mnt
> ls /mnt/dir ==> -ENOENT
:( Indeed. There are situations when no single "stable" directory might exist.
OK, it loooks like there's not much we can do about it w/o patching the kernel.
Let's get back to your original approach when you just made necessary decision
on restore side -- if the ghost file's dir(s) doesn't exist -- create one(s) and
unlink it(them) afterwards.
-- Pavel
> If we have such configuration, which directory should we consider as "stable"?
> Should we unhide /dev/YYY and restore ghost file on it?
>
> Thanks,
> Kirill
>
>>> + }
>>> +
>>> + *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