Hi Amir!

On Sun 26-06-22 18:57:01, Amir Goldstein wrote:

On Fri, Jun 24, 2022 at 5:35 PM Amir Goldstein [off-list ref] wrote:

quoted

As we discussed [1], here is the implementation of the new
FAN_MARK_IGNORE API, to try and sort the historic mess of
FAN_MARK_IGNORED_MASK.

When we started talking about adding FAN_MARK_IGNORE
it was to address one specific flaw of FAN_MARK_IGNORED_MASK,
but after staring at the API for some time, I realized there are other
wrinkles with FAN_MARK_IGNORED_MASK that could be addressed
by a fresh new API.

I added more input validations following the EEXIST that you requested.
The new errors can be seen in the ERRORS section of the man page [3].
The new restrictions will reduce the size of the test matrix, but I did not
update the LTP tests [2] to check the new restrictions yet.

I do not plan to post v3 patches before improving the LTP tests,
but I wanted to send this heads up as an API proposal review.
The kernel commit that adds FAN_MARK_IGNORE [1] summarize the
new API restrictions as follows:

    The new behavior is non-downgradable.  After calling fanotify_mark() with
    FAN_MARK_IGNORE once, calling fanotify_mark() with FAN_MARK_IGNORED_MASK
    on the same object will return EEXIST error.

    Setting the event flags with FAN_MARK_IGNORE on a non-dir inode mark
    has no meaning and will return ENOTDIR error.

    The meaning of FAN_MARK_IGNORED_SURV_MODIFY is preserved with the new
    FAN_MARK_IGNORE flag, but with a few semantic differences:

    1. FAN_MARK_IGNORED_SURV_MODIFY is required for filesystem and mount
       marks and on an inode mark on a directory. Omitting this flag
       will return EINVAL or EISDIR error.

    2. An ignore mask on a non-directory inode that survives modify could
       never be downgraded to an ignore mask that does not survive modify.
       With new FAN_MARK_IGNORE semantics we make that rule explicit -
       trying to update a surviving ignore mask without the flag
       FAN_MARK_IGNORED_SURV_MODIFY will return EEXIST error.

    The conveniene macro FAN_MARK_IGNORE_SURV is added for
    (FAN_MARK_IGNORE | FAN_MARK_IGNORED_SURV_MODIFY), because the
    common case should use short constant names.

This looks good to me. Thanks for working on this. The only additional
thing that occurred to me is that we might want to restrict events usable
with FAN_MARK_IGNORE similarly as we did restrict non-sensical events with
FAN_REPORT_TARGET_FID but after putting more thought into it I'm not sure
it is such a great idea because it is not so obvious which events may
be valid to ignore for a particular object and how is it with backward
compatibility when a new type of event becomes possible for an object.

								Honza
 

[1] https://github.com/amir73il/linux/commits/fan_mark_ignore
[2] https://github.com/amir73il/ltp/commits/fan_mark_ignore
[3] https://github.com/amir73il/man-pages/commits/fan_mark_ignore

-- 
Jan Kara [off-list ref]
SUSE Labs, CR