[PATCH] libxfs: make resync with the userspace libxfs easier

[PATCH] libxfs: make resync with the userspace libxfs easier

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Prepare to resync the userspace libxfs with the kernel libxfs.  There
were a few things I missed -- a couple of static inline directory
functions that have to be exported for xfs_repair; a couple of directory
naming functions that make porting much easier if they're /not/ static
inline; and a u16 usage that should have been uint16_t.

None of these things are bugs in their own right; this just makes
porting xfsprogs easier.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c      |    2 +-
 fs/xfs/libxfs/xfs_dir2.c      |   21 +++++++++++++++++++++
 fs/xfs/libxfs/xfs_dir2_priv.h |   29 +++++++++--------------------
 fs/xfs/libxfs/xfs_dir2_sf.c   |    6 +++---
 4 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 4a802b3abe77..4c2e046fbfad 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4561,7 +4561,7 @@ xfs_bmapi_convert_delalloc(
 	struct xfs_mount	*mp = ip->i_mount;
 	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, offset);
 	struct xfs_bmalloca	bma = { NULL };
-	u16			flags = 0;
+	uint16_t		flags = 0;
 	struct xfs_trans	*tp;
 	int			error;
 
diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index 0aa87cbde49e..dd6fcaaea318 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -724,3 +724,24 @@ xfs_dir2_namecheck(
 	/* There shouldn't be any slashes or nulls here */
 	return !memchr(name, '/', length) && !memchr(name, 0, length);
 }
+
+xfs_dahash_t
+xfs_dir2_hashname(
+	struct xfs_mount	*mp,
+	struct xfs_name		*name)
+{
+	if (unlikely(xfs_sb_version_hasasciici(&mp->m_sb)))
+		return xfs_ascii_ci_hashname(name);
+	return xfs_da_hashname(name->name, name->len);
+}
+
+enum xfs_dacmp
+xfs_dir2_compname(
+	struct xfs_da_args	*args,
+	const unsigned char	*name,
+	int			len)
+{
+	if (unlikely(xfs_sb_version_hasasciici(&args->dp->i_mount->m_sb)))
+		return xfs_ascii_ci_compname(args, name, len);
+	return xfs_da_compname(args, name, len);
+}
diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
index c031c53d0f0d..01ee0b926572 100644
--- a/fs/xfs/libxfs/xfs_dir2_priv.h
+++ b/fs/xfs/libxfs/xfs_dir2_priv.h
@@ -175,6 +175,12 @@ extern int xfs_dir2_sf_lookup(struct xfs_da_args *args);
 extern int xfs_dir2_sf_removename(struct xfs_da_args *args);
 extern int xfs_dir2_sf_replace(struct xfs_da_args *args);
 extern xfs_failaddr_t xfs_dir2_sf_verify(struct xfs_inode *ip);
+int xfs_dir2_sf_entsize(struct xfs_mount *mp,
+		struct xfs_dir2_sf_hdr *hdr, int len);
+void xfs_dir2_sf_put_ino(struct xfs_mount *mp, struct xfs_dir2_sf_hdr *hdr,
+		struct xfs_dir2_sf_entry *sfep, xfs_ino_t ino);
+void xfs_dir2_sf_put_ftype(struct xfs_mount *mp,
+		struct xfs_dir2_sf_entry *sfep, uint8_t ftype);
 
 /* xfs_dir2_readdir.c */
 extern int xfs_readdir(struct xfs_trans *tp, struct xfs_inode *dp,
@@ -194,25 +200,8 @@ xfs_dir2_data_entsize(
 	return round_up(len, XFS_DIR2_DATA_ALIGN);
 }
 
-static inline xfs_dahash_t
-xfs_dir2_hashname(
-	struct xfs_mount	*mp,
-	struct xfs_name		*name)
-{
-	if (unlikely(xfs_sb_version_hasasciici(&mp->m_sb)))
-		return xfs_ascii_ci_hashname(name);
-	return xfs_da_hashname(name->name, name->len);
-}
-
-static inline enum xfs_dacmp
-xfs_dir2_compname(
-	struct xfs_da_args	*args,
-	const unsigned char	*name,
-	int			len)
-{
-	if (unlikely(xfs_sb_version_hasasciici(&args->dp->i_mount->m_sb)))
-		return xfs_ascii_ci_compname(args, name, len);
-	return xfs_da_compname(args, name, len);
-}
+xfs_dahash_t xfs_dir2_hashname(struct xfs_mount *mp, struct xfs_name *name);
+enum xfs_dacmp xfs_dir2_compname(struct xfs_da_args *args,
+		const unsigned char *name, int len);
 
 #endif /* __XFS_DIR2_PRIV_H__ */
diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
index 8b94d33d232f..7b7f6fb2ea3b 100644
--- a/fs/xfs/libxfs/xfs_dir2_sf.c
+++ b/fs/xfs/libxfs/xfs_dir2_sf.c
@@ -37,7 +37,7 @@ static void xfs_dir2_sf_check(xfs_da_args_t *args);
 static void xfs_dir2_sf_toino4(xfs_da_args_t *args);
 static void xfs_dir2_sf_toino8(xfs_da_args_t *args);
 
-static int
+int
 xfs_dir2_sf_entsize(
 	struct xfs_mount	*mp,
 	struct xfs_dir2_sf_hdr	*hdr,
@@ -84,7 +84,7 @@ xfs_dir2_sf_get_ino(
 	return get_unaligned_be64(from) & XFS_MAXINUMBER;
 }
 
-static void
+void
 xfs_dir2_sf_put_ino(
 	struct xfs_mount		*mp,
 	struct xfs_dir2_sf_hdr		*hdr,
@@ -145,7 +145,7 @@ xfs_dir2_sf_get_ftype(
 	return XFS_DIR3_FT_UNKNOWN;
 }
 
-static void
+void
 xfs_dir2_sf_put_ftype(
 	struct xfs_mount	*mp,
 	struct xfs_dir2_sf_entry *sfep,

Re: [PATCH] libxfs: make resync with the userspace libxfs easier

[Eric Sandeen]

On 12/16/19 8:35 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Prepare to resync the userspace libxfs with the kernel libxfs.  There
> were a few things I missed -- a couple of static inline directory
> functions that have to be exported for xfs_repair; a couple of directory
> naming functions that make porting much easier if they're /not/ static
> inline; and a u16 usage that should have been uint16_t.
> 
> None of these things are bugs in their own right; this just makes
> porting xfsprogs easier.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

This all seems fine - I'm wondering about prototypes in xfs_dir2_priv.h
vs xfs_dir2.h - at this point I'm not sure why we have the two,
help?

But whatevs, if that needs cleanup it can come later.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  fs/xfs/libxfs/xfs_bmap.c      |    2 +-
>  fs/xfs/libxfs/xfs_dir2.c      |   21 +++++++++++++++++++++
>  fs/xfs/libxfs/xfs_dir2_priv.h |   29 +++++++++--------------------
>  fs/xfs/libxfs/xfs_dir2_sf.c   |    6 +++---
>  4 files changed, 34 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 4a802b3abe77..4c2e046fbfad 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4561,7 +4561,7 @@ xfs_bmapi_convert_delalloc(
>  	struct xfs_mount	*mp = ip->i_mount;
>  	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, offset);
>  	struct xfs_bmalloca	bma = { NULL };
> -	u16			flags = 0;
> +	uint16_t		flags = 0;
>  	struct xfs_trans	*tp;
>  	int			error;
>  
> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
> index 0aa87cbde49e..dd6fcaaea318 100644
> --- a/fs/xfs/libxfs/xfs_dir2.c
> +++ b/fs/xfs/libxfs/xfs_dir2.c
> @@ -724,3 +724,24 @@ xfs_dir2_namecheck(
>  	/* There shouldn't be any slashes or nulls here */
>  	return !memchr(name, '/', length) && !memchr(name, 0, length);
>  }
> +
> +xfs_dahash_t
> +xfs_dir2_hashname(
> +	struct xfs_mount	*mp,
> +	struct xfs_name		*name)
> +{
> +	if (unlikely(xfs_sb_version_hasasciici(&mp->m_sb)))
> +		return xfs_ascii_ci_hashname(name);
> +	return xfs_da_hashname(name->name, name->len);
> +}
> +
> +enum xfs_dacmp
> +xfs_dir2_compname(
> +	struct xfs_da_args	*args,
> +	const unsigned char	*name,
> +	int			len)
> +{
> +	if (unlikely(xfs_sb_version_hasasciici(&args->dp->i_mount->m_sb)))
> +		return xfs_ascii_ci_compname(args, name, len);
> +	return xfs_da_compname(args, name, len);
> +}
> diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
> index c031c53d0f0d..01ee0b926572 100644
> --- a/fs/xfs/libxfs/xfs_dir2_priv.h
> +++ b/fs/xfs/libxfs/xfs_dir2_priv.h
> @@ -175,6 +175,12 @@ extern int xfs_dir2_sf_lookup(struct xfs_da_args *args);
>  extern int xfs_dir2_sf_removename(struct xfs_da_args *args);
>  extern int xfs_dir2_sf_replace(struct xfs_da_args *args);
>  extern xfs_failaddr_t xfs_dir2_sf_verify(struct xfs_inode *ip);
> +int xfs_dir2_sf_entsize(struct xfs_mount *mp,
> +		struct xfs_dir2_sf_hdr *hdr, int len);
> +void xfs_dir2_sf_put_ino(struct xfs_mount *mp, struct xfs_dir2_sf_hdr *hdr,
> +		struct xfs_dir2_sf_entry *sfep, xfs_ino_t ino);
> +void xfs_dir2_sf_put_ftype(struct xfs_mount *mp,
> +		struct xfs_dir2_sf_entry *sfep, uint8_t ftype);
>  
>  /* xfs_dir2_readdir.c */
>  extern int xfs_readdir(struct xfs_trans *tp, struct xfs_inode *dp,
> @@ -194,25 +200,8 @@ xfs_dir2_data_entsize(
>  	return round_up(len, XFS_DIR2_DATA_ALIGN);
>  }
>  
> -static inline xfs_dahash_t
> -xfs_dir2_hashname(
> -	struct xfs_mount	*mp,
> -	struct xfs_name		*name)
> -{
> -	if (unlikely(xfs_sb_version_hasasciici(&mp->m_sb)))
> -		return xfs_ascii_ci_hashname(name);
> -	return xfs_da_hashname(name->name, name->len);
> -}
> -
> -static inline enum xfs_dacmp
> -xfs_dir2_compname(
> -	struct xfs_da_args	*args,
> -	const unsigned char	*name,
> -	int			len)
> -{
> -	if (unlikely(xfs_sb_version_hasasciici(&args->dp->i_mount->m_sb)))
> -		return xfs_ascii_ci_compname(args, name, len);
> -	return xfs_da_compname(args, name, len);
> -}
> +xfs_dahash_t xfs_dir2_hashname(struct xfs_mount *mp, struct xfs_name *name);
> +enum xfs_dacmp xfs_dir2_compname(struct xfs_da_args *args,
> +		const unsigned char *name, int len);
>  
>  #endif /* __XFS_DIR2_PRIV_H__ */
> diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
> index 8b94d33d232f..7b7f6fb2ea3b 100644
> --- a/fs/xfs/libxfs/xfs_dir2_sf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_sf.c
> @@ -37,7 +37,7 @@ static void xfs_dir2_sf_check(xfs_da_args_t *args);
>  static void xfs_dir2_sf_toino4(xfs_da_args_t *args);
>  static void xfs_dir2_sf_toino8(xfs_da_args_t *args);
>  
> -static int
> +int
>  xfs_dir2_sf_entsize(
>  	struct xfs_mount	*mp,
>  	struct xfs_dir2_sf_hdr	*hdr,
> @@ -84,7 +84,7 @@ xfs_dir2_sf_get_ino(
>  	return get_unaligned_be64(from) & XFS_MAXINUMBER;
>  }
>  
> -static void
> +void
>  xfs_dir2_sf_put_ino(
>  	struct xfs_mount		*mp,
>  	struct xfs_dir2_sf_hdr		*hdr,
> @@ -145,7 +145,7 @@ xfs_dir2_sf_get_ftype(
>  	return XFS_DIR3_FT_UNKNOWN;
>  }
>  
> -static void
> +void
>  xfs_dir2_sf_put_ftype(
>  	struct xfs_mount	*mp,
>  	struct xfs_dir2_sf_entry *sfep,
> 

Re: [PATCH] libxfs: make resync with the userspace libxfs easier

[Darrick J. Wong]

On Wed, Dec 18, 2019 at 03:00:14PM -0600, Eric Sandeen wrote:
> On 12/16/19 8:35 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Prepare to resync the userspace libxfs with the kernel libxfs.  There
> > were a few things I missed -- a couple of static inline directory
> > functions that have to be exported for xfs_repair; a couple of directory
> > naming functions that make porting much easier if they're /not/ static
> > inline; and a u16 usage that should have been uint16_t.
> > 
> > None of these things are bugs in their own right; this just makes
> > porting xfsprogs easier.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> This all seems fine - I'm wondering about prototypes in xfs_dir2_priv.h
> vs xfs_dir2.h - at this point I'm not sure why we have the two,
> help?

I think the xfs_dir2_priv header is for symbols that are internal to
directories, whereas xfs_dir2.h is for functions that higher level
functions should call to create/modify/erase directory entries.

> But whatevs, if that needs cleanup it can come later.
> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>

Thanks.

--D

> > ---
> >  fs/xfs/libxfs/xfs_bmap.c      |    2 +-
> >  fs/xfs/libxfs/xfs_dir2.c      |   21 +++++++++++++++++++++
> >  fs/xfs/libxfs/xfs_dir2_priv.h |   29 +++++++++--------------------
> >  fs/xfs/libxfs/xfs_dir2_sf.c   |    6 +++---
> >  4 files changed, 34 insertions(+), 24 deletions(-)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> > index 4a802b3abe77..4c2e046fbfad 100644
> > --- a/fs/xfs/libxfs/xfs_bmap.c
> > +++ b/fs/xfs/libxfs/xfs_bmap.c
> > @@ -4561,7 +4561,7 @@ xfs_bmapi_convert_delalloc(
> >  	struct xfs_mount	*mp = ip->i_mount;
> >  	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, offset);
> >  	struct xfs_bmalloca	bma = { NULL };
> > -	u16			flags = 0;
> > +	uint16_t		flags = 0;
> >  	struct xfs_trans	*tp;
> >  	int			error;
> >  
> > diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
> > index 0aa87cbde49e..dd6fcaaea318 100644
> > --- a/fs/xfs/libxfs/xfs_dir2.c
> > +++ b/fs/xfs/libxfs/xfs_dir2.c
> > @@ -724,3 +724,24 @@ xfs_dir2_namecheck(
> >  	/* There shouldn't be any slashes or nulls here */
> >  	return !memchr(name, '/', length) && !memchr(name, 0, length);
> >  }
> > +
> > +xfs_dahash_t
> > +xfs_dir2_hashname(
> > +	struct xfs_mount	*mp,
> > +	struct xfs_name		*name)
> > +{
> > +	if (unlikely(xfs_sb_version_hasasciici(&mp->m_sb)))
> > +		return xfs_ascii_ci_hashname(name);
> > +	return xfs_da_hashname(name->name, name->len);
> > +}
> > +
> > +enum xfs_dacmp
> > +xfs_dir2_compname(
> > +	struct xfs_da_args	*args,
> > +	const unsigned char	*name,
> > +	int			len)
> > +{
> > +	if (unlikely(xfs_sb_version_hasasciici(&args->dp->i_mount->m_sb)))
> > +		return xfs_ascii_ci_compname(args, name, len);
> > +	return xfs_da_compname(args, name, len);
> > +}
> > diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
> > index c031c53d0f0d..01ee0b926572 100644
> > --- a/fs/xfs/libxfs/xfs_dir2_priv.h
> > +++ b/fs/xfs/libxfs/xfs_dir2_priv.h
> > @@ -175,6 +175,12 @@ extern int xfs_dir2_sf_lookup(struct xfs_da_args *args);
> >  extern int xfs_dir2_sf_removename(struct xfs_da_args *args);
> >  extern int xfs_dir2_sf_replace(struct xfs_da_args *args);
> >  extern xfs_failaddr_t xfs_dir2_sf_verify(struct xfs_inode *ip);
> > +int xfs_dir2_sf_entsize(struct xfs_mount *mp,
> > +		struct xfs_dir2_sf_hdr *hdr, int len);
> > +void xfs_dir2_sf_put_ino(struct xfs_mount *mp, struct xfs_dir2_sf_hdr *hdr,
> > +		struct xfs_dir2_sf_entry *sfep, xfs_ino_t ino);
> > +void xfs_dir2_sf_put_ftype(struct xfs_mount *mp,
> > +		struct xfs_dir2_sf_entry *sfep, uint8_t ftype);
> >  
> >  /* xfs_dir2_readdir.c */
> >  extern int xfs_readdir(struct xfs_trans *tp, struct xfs_inode *dp,
> > @@ -194,25 +200,8 @@ xfs_dir2_data_entsize(
> >  	return round_up(len, XFS_DIR2_DATA_ALIGN);
> >  }
> >  
> > -static inline xfs_dahash_t
> > -xfs_dir2_hashname(
> > -	struct xfs_mount	*mp,
> > -	struct xfs_name		*name)
> > -{
> > -	if (unlikely(xfs_sb_version_hasasciici(&mp->m_sb)))
> > -		return xfs_ascii_ci_hashname(name);
> > -	return xfs_da_hashname(name->name, name->len);
> > -}
> > -
> > -static inline enum xfs_dacmp
> > -xfs_dir2_compname(
> > -	struct xfs_da_args	*args,
> > -	const unsigned char	*name,
> > -	int			len)
> > -{
> > -	if (unlikely(xfs_sb_version_hasasciici(&args->dp->i_mount->m_sb)))
> > -		return xfs_ascii_ci_compname(args, name, len);
> > -	return xfs_da_compname(args, name, len);
> > -}
> > +xfs_dahash_t xfs_dir2_hashname(struct xfs_mount *mp, struct xfs_name *name);
> > +enum xfs_dacmp xfs_dir2_compname(struct xfs_da_args *args,
> > +		const unsigned char *name, int len);
> >  
> >  #endif /* __XFS_DIR2_PRIV_H__ */
> > diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
> > index 8b94d33d232f..7b7f6fb2ea3b 100644
> > --- a/fs/xfs/libxfs/xfs_dir2_sf.c
> > +++ b/fs/xfs/libxfs/xfs_dir2_sf.c
> > @@ -37,7 +37,7 @@ static void xfs_dir2_sf_check(xfs_da_args_t *args);
> >  static void xfs_dir2_sf_toino4(xfs_da_args_t *args);
> >  static void xfs_dir2_sf_toino8(xfs_da_args_t *args);
> >  
> > -static int
> > +int
> >  xfs_dir2_sf_entsize(
> >  	struct xfs_mount	*mp,
> >  	struct xfs_dir2_sf_hdr	*hdr,
> > @@ -84,7 +84,7 @@ xfs_dir2_sf_get_ino(
> >  	return get_unaligned_be64(from) & XFS_MAXINUMBER;
> >  }
> >  
> > -static void
> > +void
> >  xfs_dir2_sf_put_ino(
> >  	struct xfs_mount		*mp,
> >  	struct xfs_dir2_sf_hdr		*hdr,
> > @@ -145,7 +145,7 @@ xfs_dir2_sf_get_ftype(
> >  	return XFS_DIR3_FT_UNKNOWN;
> >  }
> >  
> > -static void
> > +void
> >  xfs_dir2_sf_put_ftype(
> >  	struct xfs_mount	*mp,
> >  	struct xfs_dir2_sf_entry *sfep,
> > 

Re: [PATCH] libxfs: make resync with the userspace libxfs easier

[Christoph Hellwig]

On Mon, Dec 16, 2019 at 06:35:35PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Prepare to resync the userspace libxfs with the kernel libxfs.  There
> were a few things I missed -- a couple of static inline directory
> functions that have to be exported for xfs_repair; a couple of directory
> naming functions that make porting much easier if they're /not/ static
> inline; and a u16 usage that should have been uint16_t.
> 
> None of these things are bugs in their own right; this just makes
> porting xfsprogs easier.

Instead of exporting random low-level helpers can you please look
into refactoring repair to not require such low level access.  E.g.
the put_ino helper seems to be mostly used for convert short form
directories from and to the 8 byte inode format, for which we already
have kernel helpers in a slighty different form.

I'm also kinda pissed that this was rushed into mainline after -rc2
despite not fixing anything in the kernel.  That is not how the
development cycle is supposed to work.

Re: [PATCH] libxfs: make resync with the userspace libxfs easier

[Darrick J. Wong]

On Tue, Dec 24, 2019 at 12:29:54AM -0800, Christoph Hellwig wrote:
> On Mon, Dec 16, 2019 at 06:35:35PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Prepare to resync the userspace libxfs with the kernel libxfs.  There
> > were a few things I missed -- a couple of static inline directory
> > functions that have to be exported for xfs_repair; a couple of directory
> > naming functions that make porting much easier if they're /not/ static
> > inline; and a u16 usage that should have been uint16_t.
> > 
> > None of these things are bugs in their own right; this just makes
> > porting xfsprogs easier.
> 
> Instead of exporting random low-level helpers can you please look
> into refactoring repair to not require such low level access.  E.g.
> the put_ino helper seems to be mostly used for convert short form
> directories from and to the 8 byte inode format, for which we already
> have kernel helpers in a slighty different form.

We do?  I didn't find /any/ helpers to fix shortform inums and ftype.
xfs_repair directly manipulates a lot of directory structures directly
with libxfs functions.

> I'm also kinda pissed that this was rushed into mainline after -rc2
> despite not fixing anything in the kernel.  That is not how the
> development cycle is supposed to work.

Frankly, I trust you (and everyone else) to know when a kernel patchset
heading upstream is going to require a lot of non-trivial changes to
xfsprogs and to send a patchset to make whatever cleanups to xfsprogs
are necessary and then port the kernel patches.

95% of the time, the API changes are pretty minor, and Eric and I can
just figure it out between the two of us.  The directory refactoring you
sent for 5.5 turned out to involve a lot more work and I didn't want
Eric to be 100% stuck with the burden of figuring out how to apply
everything.

But maybe I should start asking submitters always to send kernel +
xfsprogs patches at the same time.  Allison's been doing that with the
deferred attr patches and it's very helpful not to have to imagine what
the userspace changes will look like.

So anyway, I am sorry for ruffling your feathers.  I am particularly bad
at handling small cleanups to smooth over xfsprogs when reviewers are
short.

--D

Re: [PATCH] libxfs: make resync with the userspace libxfs easier

[Christoph Hellwig]

On Tue, Dec 24, 2019 at 01:27:24PM -0800, Darrick J. Wong wrote:
> > Instead of exporting random low-level helpers can you please look
> > into refactoring repair to not require such low level access.  E.g.
> > the put_ino helper seems to be mostly used for convert short form
> > directories from and to the 8 byte inode format, for which we already
> > have kernel helpers in a slighty different form.
> 
> We do?  I didn't find /any/ helpers to fix shortform inums and ftype.
> xfs_repair directly manipulates a lot of directory structures directly
> with libxfs functions.

We have helpers to convert between the 4 and 7 byte ino sf format,
which sounds like something that should be reused.

> So anyway, I am sorry for ruffling your feathers.  I am particularly bad
> at handling small cleanups to smooth over xfsprogs when reviewers are
> short.

What really annoys me is not that patch - it is worth a discussion.
The problem is that you rushed it into -rc against the merge window
rules before we could even have a discussion.