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