Re: [PATCH 1/6] vfs: create vfs helper vfs_tmpfile()

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

 



On Mon, Jan 16, 2017 at 3:22 PM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> On Mon, Jan 16, 2017 at 12:19 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>> On Mon, Jan 16, 2017 at 1:00 PM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>>> On Sun, Jan 15, 2017 at 2:57 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>>>> Factor out some common vfs bits from do_tmpfile()
>>>> to be used by overlayfs for concurrent copy up.
>>>
>>> I'm wondering whether the vfs helper should do everything except the
>>> path lookup and the open: d_alloc(), ->tmpfile() and setting
>>> I_LINKABLE.   This will also aid in doing a ->tmpfile() for overlayfs.
>>>
>>
>> I started with that, but slowly trimmed it down to this minimal version.
>> First, mnt_want_write() can't be in there.
>> Then dentry * return value would be a strange deviation from other vfs_ helpers.
>
> What about lookup_one_len()?  It's not called vfs_something but that's
> beside the point, I think.
>
>> Lastly, all the open related operations are already performed by
>> ovl_path_open() and I did not want to make an exception,
>> so I resorted to doing d_alloc in ovl_alloc_tmpfile(), which becomes
>> a drop-in replacement for ovl_lookup_temp() and set LINKABLE in
>> ovl_link_tmpfile(), which becomes a drop-in replacement for ovl_do_rename().
>
> The logical place to set I_LINKABLE is in vfs_tmpfile().  You are
> arguing about overlayfs code structure, but that's mostly irrelevant
> when doing a new vfs interface.
>

What I tried to do at first is leave d_alloc inside vfs_tmpfile()
and return dentry, but I did not want this interface to deal with struct file
as well.

so do_tmpfile() is very roughly equivalent to:
path_lookupat()
mnt_want_write()
vfs_tmpfile() (including d_alloc)
dentry_open()
inode->i_state |= I_LINKABLE

So I can set LINKABLE inside vfs_tmpfile() only if I also pass
it open_flag and then it would also happen before open returns
success.

My intention was to keep the VFS patch as boring and uncontroversial
as possible.

I am tempted to say: could you possibly create that extra re-factoring yourself
either before or after this patch set? You are the one who is going to
be selling it to
Al after all...

Either that, or just send me better guidelines and I'll do the leg work myself.

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



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux