[CRIU] [PATCH 2/3] net: allow c/r of empty bridges in the container

[Tycho Andersen]

On Wed, Nov 11, 2015 at 8:36 AM, Pavel Emelyanov <xemul at parallels.com> wrote:
> On 11/11/2015 06:24 PM, Tycho Andersen wrote:
>> On Wed, Nov 11, 2015 at 03:49:32PM +0300, Pavel Emelyanov wrote:
>>> Please, find some comments inline.
>>>
>>>> --- a/mount.c
>>>> +++ b/mount.c
>>>> @@ -1357,31 +1357,19 @@ out:
>>>>  static int dump_empty_fs(struct mount_info *pm)
>>>>  {
>>>>     int fd, ret = -1;
>>>> -   struct dirent *de;
>>>> -   DIR *fdir = NULL;
>>>>     fd = open_mountpoint(pm);
>>>>
>>>>     if (fd < 0)
>>>>             return -1;
>>>>
>>>> -   fdir = fdopendir(fd);
>>>> -   if (fdir == NULL) {
>>>> -           close(fd);
>>>> +   ret = is_empty_dir(fd);
>>>> +   close(fd);
>>>> +   if (ret < 0) {
>>>> +           pr_err("%s isn't empty\n", pm->fstype->name);
>>>>             return -1;
>>>>     }
>>>>
>>>> -   while ((de = readdir(fdir))) {
>>>> -           if (dir_dots(de))
>>>> -                   continue;
>>>> -
>>>> -           pr_err("%s isn't empty: %s\n", pm->fstype->name, de->d_name);
>>>> -           goto out;
>>>> -   }
>>>> -
>>>> -   ret = 0;
>>>> -out:
>>>> -   closedir(fdir);
>>>> -   return ret;
>>>> +   return ret ? 0 : -1;
>>>
>>> You return "OK, let's dump it" when dir emptiness check fails, while
>>> before the patch error resulted in error.
>>
>> if is_empty_dir() returns 0 (i.e. the dir is not empty), this returns
>> -1 doesn't it?
>
> It does. But if is_empty_dir() returns -1, meaning "something went
> wrong" this returns 0.

The patch tests for that above (after it closes the fd); it is a
little misleading, though. I can rearrange it if there is a better
way.

Tycho

>
> -- Pavel