Re: [PATCH v2] xfs: deprecate BMV_IF_NO_DMAPI_READ flag

[Anthony Iliopoulos <ailiop@suse.com>]

On Wed, Mar 31, 2021 at 09:31:14AM -0700, Darrick J. Wong wrote:
> On Wed, Mar 31, 2021 at 06:26:16PM +0200, Anthony Iliopoulos wrote:
> > Use of the flag has had no effect since kernel commit 288699fecaff
> > ("xfs: drop dmapi hooks"), which removed all dmapi related code, so
> > deprecate it.
> > 
> > Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>
> > ---
> > changes since v1:
> >  - retain flag definition to prevent reuse and not break kabi, per
> >    Darrick's suggestion.
> > 
> >  fs/xfs/libxfs/xfs_fs.h | 4 ++--
> >  fs/xfs/xfs_ioctl.c     | 2 --
> >  2 files changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> > index 6fad140d4c8e..4ef813e00e9e 100644
> > --- a/fs/xfs/libxfs/xfs_fs.h
> > +++ b/fs/xfs/libxfs/xfs_fs.h
> > @@ -65,13 +65,13 @@ struct getbmapx {
> >  
> >  /*	bmv_iflags values - set by XFS_IOC_GETBMAPX caller.	*/
> >  #define BMV_IF_ATTRFORK		0x1	/* return attr fork rather than data */
> > -#define BMV_IF_NO_DMAPI_READ	0x2	/* Do not generate DMAPI read event  */
> > +#define BMV_IF_NO_DMAPI_READ	0x2	/* Deprecated */
> >  #define BMV_IF_PREALLOC		0x4	/* rtn status BMV_OF_PREALLOC if req */
> >  #define BMV_IF_DELALLOC		0x8	/* rtn status BMV_OF_DELALLOC if req */
> >  #define BMV_IF_NO_HOLES		0x10	/* Do not return holes */
> >  #define BMV_IF_COWFORK		0x20	/* return CoW fork rather than data */
> >  #define BMV_IF_VALID	\
> > -	(BMV_IF_ATTRFORK|BMV_IF_NO_DMAPI_READ|BMV_IF_PREALLOC|	\
> > +	(BMV_IF_ATTRFORK|BMV_IF_PREALLOC|	\
> >  	 BMV_IF_DELALLOC|BMV_IF_NO_HOLES|BMV_IF_COWFORK)
> 
> What about the xfs/296 regression that the kernel robot reported?
> 
> I /think/ that's a result of removing this flag from BMV_IF_VALID, which
> is used to reject unknown input flags from the GETBMAP caller.  In the
> current upstream the flag is valid even if it does nothing, so we have
> to preserve that behavior.

In that case then it indeed needs to stay in BMV_IF_VALID. I assume it
is not worth putting flags into a deprecation schedule, so this will
have to stay there for as long as the ioctl exists. I'll send a v3.

Regards,
Anthony