Re: [PATCH V2] ovl: Allocate anonymous devs for lowerdirs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 30, 2017 at 1:58 PM, Chandan Rajendra
<chandan@xxxxxxxxxxxxxxxxxx> wrote:
> On Tuesday, June 27, 2017 1:01:17 PM IST Chandan Rajendra wrote:
>> On Friday, June 23, 2017 7:04:57 PM IST Amir Goldstein wrote:
>> > On Fri, Jun 23, 2017 at 2:01 PM, Chandan Rajendra
>> > <chandan@xxxxxxxxxxxxxxxxxx> wrote:
>> > > For stat(2) on lowerdir non-dir entries in non-samefs case, this commit
>> > > provides unique values for st_dev. The unique values are obtained by
>> > > allocating anonymous bdevs for each of the lowerdirs in the overlayfs
>> > > instance.
>> > >
>> > > Signed-off-by: Chandan Rajendra <chandan@xxxxxxxxxxxxxxxxxx>
>> > > ---
>> >
>> > Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
>> > Tested-by: Amir Goldstein <amir73il@xxxxxxxxx>
>> >
>> > Miklos,
>> >
>> > I re-created the branch ovl-constino [1] on top of ovl-hardlinks and without
>> > the consistent dino patches.
>> >
>> > Applied Chandan's patch and resolved conflicts with my patches.
>> > Then, applied my patch to relax constant st_ino  for non-samefs on stat(2).
>> >
>> > Mutilated unionmount-testsuite layers check [2] to get over the unexpected
>> > pseudo dev and now tests pass for non samefs including constant ino
>> > verification and persistent ino verification for non-dir.
>> >
>> > Chandan,
>> >
>> > If you can fix the mutilated unionmount-testsuite check_layer(), that would be
>> > nice.
>> >
>>
>
> Hi Amir,
>
> In dentry.created(), we have
>
>         self.__upper = on_upper or not inode or inode.filetype() == "d"
>
> Do you happen to know why we mark the dentry as being present on the upperdir
> filesystem when "inode" evaluates to false?

I don't know why that was done this way, but IIUC, only way for inode
to be None is by passing None as filetype to record_file.

>
> Consider the case of /lowerdir/a/foo101 file being created in
> set_up.py. ctx.record_file() ends up creating a new 'dentry' object with no
> inode associated with it. Hence self.__upper is set to the value True. Isn't
> this incorrect since the file foo101 is actually being created on lowerdir?

IIUC, self.__upper will only be true for no_foo and no_dir, which are
created with filetype None.

>
> I added the following new test scenario to check_layer() code,
>
>    elif dentry.on_upper() and not self.config().is_samefs() and dev != self.upper_fs():
>           raise TestError(name + ": Upperdir file has incorrect dev id")
>
> In the above code snippet, dentry.on_upper() incorrectly evaluates to True and
> hence we end up raising an exception.
>
> Maybe dentry.created should have the following statement instead,
>
>      self.__upper = on_upper or (inode and inode.filetype() == "d")

That looks correct to me.
That change will only make a difference for dentries that are
dentry.is_negative(),
but this use case is already checked above in check_layer() anyway.

Thanks,
Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystems Devel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux