<p dir="ltr"><br>
On Sep 2, 2014 7:59 PM, "Pavel Emelyanov" <<a href="mailto:xemul@parallels.com">xemul@parallels.com</a>> wrote:<br>
><br>
> On 09/02/2014 06:49 PM, Andrey Vagin wrote:<br>
> > All watch descriptors are collected in a list and then<br>
> > they are written in inotify image as a repeated field.<br>
> ><br>
> > This images merge reduces the amount of image files criu<br>
> > generates and may simplify the fix of mentioned above issue.<br>
> ><br>
> > v2: use free_inotify_wd_entry() instead of xfree in dump_one_inotify()<br>
> > v3: don't leak ie.wd_entry<br>
> > v4: save the original order of watchers<br>
> > Signed-off-by: Andrey Vagin <<a href="mailto:avagin@openvz.org">avagin@openvz.org</a>><br>
> > ---<br>
> > fsnotify.c | 85 +++++++++++++++++++++++++++++++++++++++----------<br>
> > include/image-desc.h | 2 +-<br>
> > protobuf/fsnotify.proto | 1 +<br>
> > 3 files changed, 70 insertions(+), 18 deletions(-)<br>
> ><br>
> > diff --git a/fsnotify.c b/fsnotify.c<br>
> > index c9373c7..dff548c 100644<br>
> > --- a/fsnotify.c<br>
> > +++ b/fsnotify.c<br>
> > @@ -197,40 +197,68 @@ err:<br>
> > return -1;<br>
> > }<br>
> ><br>
> > +struct watch_list {<br>
> > + struct list_head list;<br>
> > + int n;<br>
> > +};<br>
> > +<br>
> > static int dump_inotify_entry(union fdinfo_entries *e, void *arg)<br>
> > {<br>
> > - InotifyWdEntry *we = &e->ify.e;<br>
> > - int ret = -1;<br>
> > + struct watch_list *wd_list = (struct watch_list *) arg;<br>
> > + struct inotify_wd_entry *wd_entry = (struct inotify_wd_entry *) e;;<br>
> > + InotifyWdEntry *we = &wd_entry->e;<br>
> ><br>
> > - we->id = *(u32 *)arg;<br>
> > pr_info("wd: wd 0x%08x s_dev 0x%08x i_ino 0x%16"PRIx64" mask 0x%08x\n",<br>
> > we->wd, we->s_dev, we->i_ino, we->mask);<br>
> > pr_info("\t[fhandle] bytes 0x%08x type 0x%08x __handle 0x%016"PRIx64":0x%016"PRIx64"\n",<br>
> > we->f_handle->bytes, we->f_handle->type,<br>
> > we->f_handle->handle[0], we->f_handle->handle[1]);<br>
> ><br>
> > - if (check_open_handle(we->s_dev, we->i_ino, we->f_handle))<br>
> > - goto out;<br>
> > + if (check_open_handle(we->s_dev, we->i_ino, we->f_handle)) {<br>
> > + free_inotify_wd_entry(e);<br>
> > + return -1;<br>
> > + }<br>
> ><br>
> > - ret = pb_write_one(fdset_fd(glob_fdset, CR_FD_INOTIFY_WD), we, PB_INOTIFY_WD);<br>
> > -out:<br>
> > - free_inotify_wd_entry(e);<br>
> > - return ret;<br>
> > + list_add_tail(&wd_entry->node, &wd_list->list);<br>
> > + wd_list->n++;<br>
> > +<br>
> > + return 0;<br>
> > }<br>
> ><br>
> > static int dump_one_inotify(int lfd, u32 id, const struct fd_parms *p)<br>
> > {<br>
> > + struct watch_list wd_list = {LIST_HEAD_INIT(wd_list.list), 0};<br>
> > InotifyFileEntry ie = INOTIFY_FILE_ENTRY__INIT;<br>
> > + union fdinfo_entries *we, *tmp;<br>
> > + int exit_code = -1, i;<br>
> ><br>
> > <a href="http://ie.id">ie.id</a> = id;<br>
> > ie.flags = p->flags;<br>
> > ie.fown = (FownEntry *)&p->fown;<br>
> ><br>
> > + if (parse_fdinfo(lfd, FD_TYPES__INOTIFY, dump_inotify_entry, &wd_list))<br>
> > + goto free;<br>
> > +<br>
> > + ie.wd = xmalloc(sizeof(*ie.wd) * wd_list.n);</p>
<p dir="ltr">wd_list.n is used here</p>
<p dir="ltr">> > + if (!ie.wd)<br>
> > + goto free;<br>
> > +<br>
> > + i = 0;<br>
> > + list_for_each_entry(we, &wd_list.list, ify.node)<br>
> > + ie.wd[i++] = &we->ify.e;<br>
> > + ie.n_wd = wd_list.n;<br>
><br>
> Yet again. Why do we need this wd_list.n? The i variable has<br>
> the number we need at that place.</p>
<p dir="ltr">It is used for allocating the ie.wd array</p>
<p dir="ltr">><br>
> > +<br>
> > pr_info("id 0x%08x flags 0x%08x\n", <a href="http://ie.id">ie.id</a>, ie.flags);<br>
> > if (pb_write_one(fdset_fd(glob_fdset, CR_FD_INOTIFY_FILE), &ie, PB_INOTIFY_FILE))<br>
> > - return -1;<br>
> > + goto free;<br>
> ><br>
> > - return parse_fdinfo(lfd, FD_TYPES__INOTIFY, dump_inotify_entry, &id);<br>
> > + exit_code = 0;<br>
> > +free:<br>
> > + xfree(ie.wd);<br>
> > + list_for_each_entry_safe(we, tmp, &wd_list.list, ify.node)<br>
> > + free_inotify_wd_entry(we);<br>
> > +<br>
> > + return exit_code;<br>
> > }<br>
> ><br>
> > static int pre_dump_inotify_entry(union fdinfo_entries *e, void *arg)<br>
> > @@ -613,15 +641,10 @@ static struct fsnotify_file_info *find_inotify_info(unsigned id)<br>
> > return NULL;<br>
> > }<br>
> ><br>
> > -static int collect_inotify_mark(struct fsnotify_mark_info *mark)<br>
> > +static int __collect_inotify_mark(struct fsnotify_file_info *p, struct fsnotify_mark_info *mark)<br>
> > {<br>
> > - struct fsnotify_file_info *p;<br>
> > struct fsnotify_mark_info *m;<br>
> ><br>
> > - p = find_inotify_info(mark->iwe->id);<br>
> > - if (!p)<br>
> > - return -1;<br>
> > -<br>
> > /*<br>
> > * We should put marks in wd ascending order. See comment<br>
> > * in restore_one_inotify() for explanation.<br>
> > @@ -635,6 +658,17 @@ static int collect_inotify_mark(struct fsnotify_mark_info *mark)<br>
> > return 0;<br>
> > }<br>
> ><br>
> > +static int collect_inotify_mark(struct fsnotify_mark_info *mark)<br>
> > +{<br>
> > + struct fsnotify_file_info *p;<br>
> > +<br>
> > + p = find_inotify_info(mark->iwe->id);<br>
> > + if (!p)<br>
> > + return -1;<br>
> > +<br>
> > + return __collect_inotify_mark(p, mark);<br>
> > +}<br>
> > +<br>
> > static int collect_fanotify_mark(struct fsnotify_mark_info *mark)<br>
> > {<br>
> > struct fsnotify_file_info *p;<br>
> > @@ -656,11 +690,28 @@ static int collect_fanotify_mark(struct fsnotify_mark_info *mark)<br>
> > static int collect_one_inotify(void *o, ProtobufCMessage *msg)<br>
> > {<br>
> > struct fsnotify_file_info *info = o;<br>
> > + int i;<br>
> ><br>
> > info->ife = pb_msg(msg, InotifyFileEntry);<br>
> > INIT_LIST_HEAD(&info->marks);<br>
> > list_add(&info->list, &inotify_info_head);<br>
> > pr_info("Collected id 0x%08x flags 0x%08x\n", info->ife->id, info->ife->flags);<br>
> > +<br>
> > + for (i = 0; i < info->ife->n_wd; i++) {<br>
> > + struct fsnotify_mark_info *mark;<br>
> > +<br>
> > + mark = xmalloc(sizeof(*mark));<br>
> > + if (!mark)<br>
> > + return -1;<br>
> > +<br>
> > + mark->iwe = info->ife->wd[i];<br>
> > + INIT_LIST_HEAD(&mark->list);<br>
> > + mark->remap = NULL;<br>
> > +<br>
> > + if (__collect_inotify_mark(info, mark))<br>
> > + return -1;<br>
> > + }<br>
> > +<br>
> > return file_desc_add(&info->d, info->ife->id, &inotify_desc_ops);<br>
> > }<br>
> ><br>
> > diff --git a/include/image-desc.h b/include/image-desc.h<br>
> > index ab592a1..75e851c 100644<br>
> > --- a/include/image-desc.h<br>
> > +++ b/include/image-desc.h<br>
> > @@ -68,7 +68,6 @@ enum {<br>
> > CR_FD_EVENTPOLL_TFD,<br>
> > CR_FD_SIGNALFD,<br>
> > CR_FD_INOTIFY_FILE,<br>
> > - CR_FD_INOTIFY_WD,<br>
> > CR_FD_FANOTIFY_FILE,<br>
> > CR_FD_FANOTIFY_MARK,<br>
> > CR_FD_TUNFILE,<br>
> > @@ -93,6 +92,7 @@ enum {<br>
> ><br>
> > CR_FD_SIGNAL,<br>
> > CR_FD_PSIGNAL,<br>
> > + CR_FD_INOTIFY_WD,<br>
> ><br>
> > CR_FD_MAX<br>
> > };<br>
> > diff --git a/protobuf/fsnotify.proto b/protobuf/fsnotify.proto<br>
> > index 073fa79..b3fc080 100644<br>
> > --- a/protobuf/fsnotify.proto<br>
> > +++ b/protobuf/fsnotify.proto<br>
> > @@ -15,6 +15,7 @@ message inotify_file_entry {<br>
> > required uint32 id = 1;<br>
> > required uint32 flags = 2;<br>
> > required fown_entry fown = 4;<br>
> > + repeated inotify_wd_entry wd = 5;<br>
> > }<br>
> ><br>
> > enum mark_type {<br>
> ><br>
><br>
</p>