Re: [PATCH v3 4.17] inotify: Add flag IN_EXCL_ADD for inotify_add_watch()

[Amir Goldstein <amir73il@gmail.com>]

On Thu, May 31, 2018 at 11:22 AM, Henry Wilson <henry.wilson@acentic.com> wrote:
>
>
> On 30/05/18 17:04, Jan Kara wrote:
>>
>> On Wed 30-05-18 18:40:27, Amir Goldstein wrote:
>>>
>>> On Wed, May 30, 2018 at 4:35 PM, Henry Wilson <henry.wilson@acentic.com>
>>> wrote:
>>>>
>>>> On 30/05/18 14:01, Jan Kara wrote:
>>>>>
>>>>>
>>>>> Thanks. The patch looks good. I've added it to my tree. BTW, do you
>>>>> plan
>>>>> on
>>>>> working on a similar addition to fanotify?
>>>>>
>>>>>                                                                  Honza
>>>>>
>>>>
>>>> Ah that's grand, I'm glad to have helped to improve things.
>>>> I'm not familiar with fanotify, however a quick look at fanotify_user.c
>>>> suggests that a similar approach may be taken by modifying:
>>>>
>>>> if(!fsn_mark) {
>>>>          ...
>>>> }
>>>> else if (create) {
>>>>          return -EEXIST;
>>>> }
>>>>
>>>> in both fanotify_add_vfsmount_mark() and fanotify_add_inode_mark()
>>>>
>>>
>>> I think that was a yes/no question and I interpret your answer as no?? >>
>>> Anyway, another yes/no question:
>>> Can you write a simple LTP test to verify the new API?
>
>
> I shall have a go at writing a test, yes.

Great.

I advice you start by forking testcases/kernel/syscalls/inotify/inotify01.c
I would verify that:
A. IN_MASK_CREATE creates a watch and events get generated
(minor flavor of inotify01.c)
B. IN_MASK_CREATE returns EEXISTS on second call for same file
C. existing mask is not modified by this attempt (events get generated
according to original mask)
D. IN_MASK_CREATE && IN_MASK_ADD return EINVAL

B-D. can be just extra steps in setup().
Do keep in mind to exit test with tst_brk(TCONF in case call
with IN_MASK_CREATE returns EINVAL


>
>>>
>>> I reccon Jan was also expecting an actual patch posted to man pages
>>> maintainer (and linux-api, which was not cc'ed on the latest patch).
>
>
> Ah, I did not know linux-api needed to be cc'ed in.
>
>>
>> Yes, and I think Henry is about to post it, just didn't get to it yet.
>
>
> For reference here is an archive link to the thread on the linux-man archive
> https://marc.info/?l=linux-man&m=152769572917930&w=2
>

OK, so I see now with the flag name change documentation patch is
out of date, but also the statement (since Linux 4.17) is incorrect

Honestly, I don't know how Michael coordinates man pages updates vs.
merged kernel patches, but the more info you provide to Michael
about were the kernel patch stands in the process is probably for the best.

Thanks,
Amir.