[CRIU] [PATCH 2/2] files: Fix breaking of fds list ordering by splice

[Pavel Begunkov]

Kirill, thanks for the explanations.

I need to get fdinfo by fd to restore file leases. So I found
find_used_fd from the files.h, which doesn't work because of the list
ordering. Seems originally it's not restricted to any particular
stage. I wonder, if it was just not documented or actually a bug.

On Sat, Jul 1, 2017 at 12:02 AM, Kirill Tkhai <ktkhai at virtuozzo.com> wrote:
> Hi, Pavel,
>
> On Fri, Jun 30, 2017 at 22:56, Pavel Begunkov wrote:
>> Kirill, could you pls take a look?
>>
>> The basic idea is to preserve the order of the final list. The
>> 'completed' list is split into 2 separate lists by #1a945c9, but the
>> final splicing may break the order. As a simplest solution, I just
>> returned it to the single list version.
>> Should they really be explicitly separated? Then there should be some
>> actual merge. Or maybe it's already sorted?
>
> *completed* list is not ordered, because we add fles there
> after they are restored. But they restore asynchronous
> and it's impossible to guess, which time the certain fle will
> be completed:
>
> if you have 3 fles in the list hashed in the order: A, B, C;
> you may have B restored first, then A, and then C.
>
> So, if we kill fake list, fles still won't be ordered. And
> I don't think we should have them be so, as we do not
> use fles after the function is finished.
>
>> On Fri, Jun 30, 2017 at 10:29 PM, Pavel Begunkov <asml.silence at gmail.com> wrote:
>> > fds lists should be sorted in ascending order, splicing of 2 (ordered)
>> > lists violates this invariant.
>> >
>> > Signed-off-by: Pavel Begunkov <asml.silence at gmail.com>
>> > ---
>> >  criu/files.c | 14 +++++---------
>> >  1 file changed, 5 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/criu/files.c b/criu/files.c
>> > index 4a263c438..a0df8f531 100644
>> > --- a/criu/files.c
>> > +++ b/criu/files.c
>> > @@ -1161,12 +1161,13 @@ static int receive_fd(struct fdinfo_list_entry *fle)
>> >         return 0;
>> >  }
>> >
>> > -static void close_fdinfos(struct list_head *list)
>> > +static void close_fake_fdinfos(struct list_head *list)
>> >  {
>> >         struct fdinfo_list_entry *fle;
>> >
>> >         list_for_each_entry(fle, list, ps_list)
>> > -               close(fle->fe->fd);
>> > +               if (fle->fake)
>> > +                       close(fle->fe->fd);
>> >  }
>> >
>> >  static int open_fdinfos(struct pstree_item *me)
>> > @@ -1174,7 +1175,6 @@ static int open_fdinfos(struct pstree_item *me)
>> >         struct list_head *list = &rsti(me)->fds;
>> >         struct fdinfo_list_entry *fle, *tmp;
>> >         LIST_HEAD(completed);
>> > -       LIST_HEAD(fake);
>> >         bool progress, again;
>> >         int st, ret = 0;
>> >
>> > @@ -1197,10 +1197,7 @@ static int open_fdinfos(struct pstree_item *me)
>> >                                  * and reduce number of fles in their checks.
>> >                                  */
>> >                                 list_del(&fle->ps_list);
>> > -                               if (!fle->fake)
>> > -                                       list_add_tail(&fle->ps_list, &completed);
>> > -                               else
>> > -                                       list_add_tail(&fle->ps_list, &fake);
>> > +                               list_add_tail(&fle->ps_list, &completed);
>> >                         }
>> >                         if (ret == 1)
>> >                                again = true;
>> > @@ -1214,9 +1211,8 @@ static int open_fdinfos(struct pstree_item *me)
>> >          * Fake fles may be used for restore other
>> >          * file types, so their closing is delayed.
>> >          */
>> > -       close_fdinfos(&fake);
>> > +       close_fake_fdinfos(&completed);
>> >  splice:
>> > -       list_splice(&fake, list);
>> >         list_splice(&completed, list);
>> >
>> >         return ret;
>> > --
>> > 2.11.1
>> >