diff mbox series

[V2] xfsprogs: cosmetic changes to libxfs_inode_alloc

Message ID 3fa15760-2e68-2c64-3914-fafbdd0e41fd@redhat.com (mailing list archive)
State Accepted
Headers show
Series [V2] xfsprogs: cosmetic changes to libxfs_inode_alloc | expand

Commit Message

Eric Sandeen Jan. 7, 2021, 7:20 p.m. UTC
This pre-patch helps make the next libxfs-sync for 5.11 a bit
more clear.

In reality, the libxfs_inode_alloc function matches the kernel's
xfs_dir_ialloc so rename it for clarity before the rest of the
sync, and change several variable names for the same reason.

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

V2: Fix up local transaction pointer problems pointed out by Brian.

Essentially, use tp locally, and reassign tpp on return.

Comments

Darrick J. Wong Jan. 7, 2021, 7:37 p.m. UTC | #1
On Thu, Jan 07, 2021 at 01:20:49PM -0600, Eric Sandeen wrote:
> This pre-patch helps make the next libxfs-sync for 5.11 a bit
> more clear.
> 
> In reality, the libxfs_inode_alloc function matches the kernel's
> xfs_dir_ialloc so rename it for clarity before the rest of the
> sync, and change several variable names for the same reason.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> V2: Fix up local transaction pointer problems pointed out by Brian.
> 
> Essentially, use tp locally, and reassign tpp on return.
> 
> diff --git a/include/xfs_inode.h b/include/xfs_inode.h
> index 742aebc8..01a62daa 100644
> --- a/include/xfs_inode.h
> +++ b/include/xfs_inode.h
> @@ -156,7 +156,7 @@ typedef struct cred {
>  	gid_t	cr_gid;
>  } cred_t;
>  
> -extern int	libxfs_inode_alloc (struct xfs_trans **, struct xfs_inode *,
> +extern int	libxfs_dir_ialloc (struct xfs_trans **, struct xfs_inode *,
>  				mode_t, nlink_t, xfs_dev_t, struct cred *,
>  				struct fsxattr *, struct xfs_inode **);
>  extern void	libxfs_trans_inode_alloc_buf (struct xfs_trans *,
> diff --git a/libxfs/util.c b/libxfs/util.c
> index 252cf91e..376c5dac 100644
> --- a/libxfs/util.c
> +++ b/libxfs/util.c
> @@ -531,9 +531,9 @@ error0:	/* Cancel bmap, cancel trans */
>   * other in repair - now there is just the one.
>   */
>  int
> -libxfs_inode_alloc(
> -	xfs_trans_t	**tp,
> -	xfs_inode_t	*pip,
> +libxfs_dir_ialloc(
> +	xfs_trans_t	**tpp,
> +	xfs_inode_t	*dp,
>  	mode_t		mode,
>  	nlink_t		nlink,
>  	xfs_dev_t	rdev,
> @@ -541,16 +541,18 @@ libxfs_inode_alloc(
>  	struct fsxattr	*fsx,
>  	xfs_inode_t	**ipp)
>  {
> -	xfs_buf_t	*ialloc_context;
> +	xfs_trans_t	*tp;
>  	xfs_inode_t	*ip;
> -	int		error;
> +	xfs_buf_t	*ialloc_context = NULL;
> +	int		code;

Maybe de-typedef this function too?  Though I guess if the next patch is
a backport of "xfs: move on-disk inode allocation out of xfs_ialloc" to
libxfs/util.c then maybe that doesn't matter and I'll just shut up.  :)

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

>  
> -	ialloc_context = (xfs_buf_t *)0;
> -	error = libxfs_ialloc(*tp, pip, mode, nlink, rdev, cr, fsx,
> +	tp = *tpp;
> +
> +	code = libxfs_ialloc(tp, dp, mode, nlink, rdev, cr, fsx,
>  			   &ialloc_context, &ip);
> -	if (error) {
> +	if (code) {
>  		*ipp = NULL;
> -		return error;
> +		return code;
>  	}
>  	if (!ialloc_context && !ip) {
>  		*ipp = NULL;
> @@ -559,25 +561,29 @@ libxfs_inode_alloc(
>  
>  	if (ialloc_context) {
>  
> -		xfs_trans_bhold(*tp, ialloc_context);
> +		xfs_trans_bhold(tp, ialloc_context);
>  
> -		error = xfs_trans_roll(tp);
> -		if (error) {
> +		code = xfs_trans_roll(&tp);
> +		if (code) {
>  			fprintf(stderr, _("%s: cannot duplicate transaction: %s\n"),
> -				progname, strerror(error));
> +				progname, strerror(code));
>  			exit(1);
>  		}
> -		xfs_trans_bjoin(*tp, ialloc_context);
> -		error = libxfs_ialloc(*tp, pip, mode, nlink, rdev, cr,
> +		xfs_trans_bjoin(tp, ialloc_context);
> +		code = libxfs_ialloc(tp, dp, mode, nlink, rdev, cr,
>  				   fsx, &ialloc_context, &ip);
>  		if (!ip)
> -			error = -ENOSPC;
> -		if (error)
> -			return error;
> +			code = -ENOSPC;
> +		if (code) {
> +			*tpp = tp;
> +			*ipp = NULL;
> +			return code;
> +		}
>  	}
>  
>  	*ipp = ip;
> -	return error;
> +	*tpp = tp;
> +	return code;
>  }
>  
>  void
> diff --git a/mkfs/proto.c b/mkfs/proto.c
> index 0fa6ffb0..8439efc4 100644
> --- a/mkfs/proto.c
> +++ b/mkfs/proto.c
> @@ -453,7 +453,7 @@ parseproto(
>  	case IF_REGULAR:
>  		buf = newregfile(pp, &len);
>  		tp = getres(mp, XFS_B_TO_FSB(mp, len));
> -		error = -libxfs_inode_alloc(&tp, pip, mode|S_IFREG, 1, 0,
> +		error = -libxfs_dir_ialloc(&tp, pip, mode|S_IFREG, 1, 0,
>  					   &creds, fsxp, &ip);
>  		if (error)
>  			fail(_("Inode allocation failed"), error);
> @@ -477,7 +477,7 @@ parseproto(
>  		}
>  		tp = getres(mp, XFS_B_TO_FSB(mp, llen));
>  
> -		error = -libxfs_inode_alloc(&tp, pip, mode|S_IFREG, 1, 0,
> +		error = -libxfs_dir_ialloc(&tp, pip, mode|S_IFREG, 1, 0,
>  					  &creds, fsxp, &ip);
>  		if (error)
>  			fail(_("Inode pre-allocation failed"), error);
> @@ -498,7 +498,7 @@ parseproto(
>  		tp = getres(mp, 0);
>  		majdev = getnum(getstr(pp), 0, 0, false);
>  		mindev = getnum(getstr(pp), 0, 0, false);
> -		error = -libxfs_inode_alloc(&tp, pip, mode|S_IFBLK, 1,
> +		error = -libxfs_dir_ialloc(&tp, pip, mode|S_IFBLK, 1,
>  				IRIX_MKDEV(majdev, mindev), &creds, fsxp, &ip);
>  		if (error) {
>  			fail(_("Inode allocation failed"), error);
> @@ -513,7 +513,7 @@ parseproto(
>  		tp = getres(mp, 0);
>  		majdev = getnum(getstr(pp), 0, 0, false);
>  		mindev = getnum(getstr(pp), 0, 0, false);
> -		error = -libxfs_inode_alloc(&tp, pip, mode|S_IFCHR, 1,
> +		error = -libxfs_dir_ialloc(&tp, pip, mode|S_IFCHR, 1,
>  				IRIX_MKDEV(majdev, mindev), &creds, fsxp, &ip);
>  		if (error)
>  			fail(_("Inode allocation failed"), error);
> @@ -525,7 +525,7 @@ parseproto(
>  
>  	case IF_FIFO:
>  		tp = getres(mp, 0);
> -		error = -libxfs_inode_alloc(&tp, pip, mode|S_IFIFO, 1, 0,
> +		error = -libxfs_dir_ialloc(&tp, pip, mode|S_IFIFO, 1, 0,
>  				&creds, fsxp, &ip);
>  		if (error)
>  			fail(_("Inode allocation failed"), error);
> @@ -537,7 +537,7 @@ parseproto(
>  		buf = getstr(pp);
>  		len = (int)strlen(buf);
>  		tp = getres(mp, XFS_B_TO_FSB(mp, len));
> -		error = -libxfs_inode_alloc(&tp, pip, mode|S_IFLNK, 1, 0,
> +		error = -libxfs_dir_ialloc(&tp, pip, mode|S_IFLNK, 1, 0,
>  				&creds, fsxp, &ip);
>  		if (error)
>  			fail(_("Inode allocation failed"), error);
> @@ -548,7 +548,7 @@ parseproto(
>  		break;
>  	case IF_DIRECTORY:
>  		tp = getres(mp, 0);
> -		error = -libxfs_inode_alloc(&tp, pip, mode|S_IFDIR, 1, 0,
> +		error = -libxfs_dir_ialloc(&tp, pip, mode|S_IFDIR, 1, 0,
>  				&creds, fsxp, &ip);
>  		if (error)
>  			fail(_("Inode allocation failed"), error);
> @@ -640,7 +640,7 @@ rtinit(
>  
>  	memset(&creds, 0, sizeof(creds));
>  	memset(&fsxattrs, 0, sizeof(fsxattrs));
> -	error = -libxfs_inode_alloc(&tp, NULL, S_IFREG, 1, 0,
> +	error = -libxfs_dir_ialloc(&tp, NULL, S_IFREG, 1, 0,
>  					&creds, &fsxattrs, &rbmip);
>  	if (error) {
>  		fail(_("Realtime bitmap inode allocation failed"), error);
> @@ -657,7 +657,7 @@ rtinit(
>  	libxfs_trans_log_inode(tp, rbmip, XFS_ILOG_CORE);
>  	libxfs_log_sb(tp);
>  	mp->m_rbmip = rbmip;
> -	error = -libxfs_inode_alloc(&tp, NULL, S_IFREG, 1, 0,
> +	error = -libxfs_dir_ialloc(&tp, NULL, S_IFREG, 1, 0,
>  					&creds, &fsxattrs, &rsumip);
>  	if (error) {
>  		fail(_("Realtime summary inode allocation failed"), error);
> diff --git a/repair/phase6.c b/repair/phase6.c
> index 682356f0..f69afac9 100644
> --- a/repair/phase6.c
> +++ b/repair/phase6.c
> @@ -919,7 +919,7 @@ mk_orphanage(xfs_mount_t *mp)
>  		do_error(_("%d - couldn't iget root inode to make %s\n"),
>  			i, ORPHANAGE);*/
>  
> -	error = -libxfs_inode_alloc(&tp, pip, mode|S_IFDIR,
> +	error = -libxfs_dir_ialloc(&tp, pip, mode|S_IFDIR,
>  					1, 0, &zerocr, &zerofsx, &ip);
>  	if (error) {
>  		do_error(_("%s inode allocation failed %d\n"),
>
Eric Sandeen Jan. 7, 2021, 7:39 p.m. UTC | #2
On 1/7/21 1:37 PM, Darrick J. Wong wrote:
> On Thu, Jan 07, 2021 at 01:20:49PM -0600, Eric Sandeen wrote:
>> This pre-patch helps make the next libxfs-sync for 5.11 a bit
>> more clear.
>>
>> In reality, the libxfs_inode_alloc function matches the kernel's
>> xfs_dir_ialloc so rename it for clarity before the rest of the
>> sync, and change several variable names for the same reason.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> V2: Fix up local transaction pointer problems pointed out by Brian.
>>
>> Essentially, use tp locally, and reassign tpp on return.
>>
>> diff --git a/include/xfs_inode.h b/include/xfs_inode.h
>> index 742aebc8..01a62daa 100644
>> --- a/include/xfs_inode.h
>> +++ b/include/xfs_inode.h
>> @@ -156,7 +156,7 @@ typedef struct cred {
>>  	gid_t	cr_gid;
>>  } cred_t;
>>  
>> -extern int	libxfs_inode_alloc (struct xfs_trans **, struct xfs_inode *,
>> +extern int	libxfs_dir_ialloc (struct xfs_trans **, struct xfs_inode *,
>>  				mode_t, nlink_t, xfs_dev_t, struct cred *,
>>  				struct fsxattr *, struct xfs_inode **);
>>  extern void	libxfs_trans_inode_alloc_buf (struct xfs_trans *,
>> diff --git a/libxfs/util.c b/libxfs/util.c
>> index 252cf91e..376c5dac 100644
>> --- a/libxfs/util.c
>> +++ b/libxfs/util.c
>> @@ -531,9 +531,9 @@ error0:	/* Cancel bmap, cancel trans */
>>   * other in repair - now there is just the one.
>>   */
>>  int
>> -libxfs_inode_alloc(
>> -	xfs_trans_t	**tp,
>> -	xfs_inode_t	*pip,
>> +libxfs_dir_ialloc(
>> +	xfs_trans_t	**tpp,
>> +	xfs_inode_t	*dp,
>>  	mode_t		mode,
>>  	nlink_t		nlink,
>>  	xfs_dev_t	rdev,
>> @@ -541,16 +541,18 @@ libxfs_inode_alloc(
>>  	struct fsxattr	*fsx,
>>  	xfs_inode_t	**ipp)
>>  {
>> -	xfs_buf_t	*ialloc_context;
>> +	xfs_trans_t	*tp;
>>  	xfs_inode_t	*ip;
>> -	int		error;
>> +	xfs_buf_t	*ialloc_context = NULL;
>> +	int		code;
> 
> Maybe de-typedef this function too?  Though I guess if the next patch is
> a backport of "xfs: move on-disk inode allocation out of xfs_ialloc" to
> libxfs/util.c then maybe that doesn't matter and I'll just shut up.  :)

As Mr. Reznor says, "everything goes away, in the end."
 
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks,
-Eric
diff mbox series

Patch

diff --git a/include/xfs_inode.h b/include/xfs_inode.h
index 742aebc8..01a62daa 100644
--- a/include/xfs_inode.h
+++ b/include/xfs_inode.h
@@ -156,7 +156,7 @@  typedef struct cred {
 	gid_t	cr_gid;
 } cred_t;
 
-extern int	libxfs_inode_alloc (struct xfs_trans **, struct xfs_inode *,
+extern int	libxfs_dir_ialloc (struct xfs_trans **, struct xfs_inode *,
 				mode_t, nlink_t, xfs_dev_t, struct cred *,
 				struct fsxattr *, struct xfs_inode **);
 extern void	libxfs_trans_inode_alloc_buf (struct xfs_trans *,
diff --git a/libxfs/util.c b/libxfs/util.c
index 252cf91e..376c5dac 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -531,9 +531,9 @@  error0:	/* Cancel bmap, cancel trans */
  * other in repair - now there is just the one.
  */
 int
-libxfs_inode_alloc(
-	xfs_trans_t	**tp,
-	xfs_inode_t	*pip,
+libxfs_dir_ialloc(
+	xfs_trans_t	**tpp,
+	xfs_inode_t	*dp,
 	mode_t		mode,
 	nlink_t		nlink,
 	xfs_dev_t	rdev,
@@ -541,16 +541,18 @@  libxfs_inode_alloc(
 	struct fsxattr	*fsx,
 	xfs_inode_t	**ipp)
 {
-	xfs_buf_t	*ialloc_context;
+	xfs_trans_t	*tp;
 	xfs_inode_t	*ip;
-	int		error;
+	xfs_buf_t	*ialloc_context = NULL;
+	int		code;
 
-	ialloc_context = (xfs_buf_t *)0;
-	error = libxfs_ialloc(*tp, pip, mode, nlink, rdev, cr, fsx,
+	tp = *tpp;
+
+	code = libxfs_ialloc(tp, dp, mode, nlink, rdev, cr, fsx,
 			   &ialloc_context, &ip);
-	if (error) {
+	if (code) {
 		*ipp = NULL;
-		return error;
+		return code;
 	}
 	if (!ialloc_context && !ip) {
 		*ipp = NULL;
@@ -559,25 +561,29 @@  libxfs_inode_alloc(
 
 	if (ialloc_context) {
 
-		xfs_trans_bhold(*tp, ialloc_context);
+		xfs_trans_bhold(tp, ialloc_context);
 
-		error = xfs_trans_roll(tp);
-		if (error) {
+		code = xfs_trans_roll(&tp);
+		if (code) {
 			fprintf(stderr, _("%s: cannot duplicate transaction: %s\n"),
-				progname, strerror(error));
+				progname, strerror(code));
 			exit(1);
 		}
-		xfs_trans_bjoin(*tp, ialloc_context);
-		error = libxfs_ialloc(*tp, pip, mode, nlink, rdev, cr,
+		xfs_trans_bjoin(tp, ialloc_context);
+		code = libxfs_ialloc(tp, dp, mode, nlink, rdev, cr,
 				   fsx, &ialloc_context, &ip);
 		if (!ip)
-			error = -ENOSPC;
-		if (error)
-			return error;
+			code = -ENOSPC;
+		if (code) {
+			*tpp = tp;
+			*ipp = NULL;
+			return code;
+		}
 	}
 
 	*ipp = ip;
-	return error;
+	*tpp = tp;
+	return code;
 }
 
 void
diff --git a/mkfs/proto.c b/mkfs/proto.c
index 0fa6ffb0..8439efc4 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -453,7 +453,7 @@  parseproto(
 	case IF_REGULAR:
 		buf = newregfile(pp, &len);
 		tp = getres(mp, XFS_B_TO_FSB(mp, len));
-		error = -libxfs_inode_alloc(&tp, pip, mode|S_IFREG, 1, 0,
+		error = -libxfs_dir_ialloc(&tp, pip, mode|S_IFREG, 1, 0,
 					   &creds, fsxp, &ip);
 		if (error)
 			fail(_("Inode allocation failed"), error);
@@ -477,7 +477,7 @@  parseproto(
 		}
 		tp = getres(mp, XFS_B_TO_FSB(mp, llen));
 
-		error = -libxfs_inode_alloc(&tp, pip, mode|S_IFREG, 1, 0,
+		error = -libxfs_dir_ialloc(&tp, pip, mode|S_IFREG, 1, 0,
 					  &creds, fsxp, &ip);
 		if (error)
 			fail(_("Inode pre-allocation failed"), error);
@@ -498,7 +498,7 @@  parseproto(
 		tp = getres(mp, 0);
 		majdev = getnum(getstr(pp), 0, 0, false);
 		mindev = getnum(getstr(pp), 0, 0, false);
-		error = -libxfs_inode_alloc(&tp, pip, mode|S_IFBLK, 1,
+		error = -libxfs_dir_ialloc(&tp, pip, mode|S_IFBLK, 1,
 				IRIX_MKDEV(majdev, mindev), &creds, fsxp, &ip);
 		if (error) {
 			fail(_("Inode allocation failed"), error);
@@ -513,7 +513,7 @@  parseproto(
 		tp = getres(mp, 0);
 		majdev = getnum(getstr(pp), 0, 0, false);
 		mindev = getnum(getstr(pp), 0, 0, false);
-		error = -libxfs_inode_alloc(&tp, pip, mode|S_IFCHR, 1,
+		error = -libxfs_dir_ialloc(&tp, pip, mode|S_IFCHR, 1,
 				IRIX_MKDEV(majdev, mindev), &creds, fsxp, &ip);
 		if (error)
 			fail(_("Inode allocation failed"), error);
@@ -525,7 +525,7 @@  parseproto(
 
 	case IF_FIFO:
 		tp = getres(mp, 0);
-		error = -libxfs_inode_alloc(&tp, pip, mode|S_IFIFO, 1, 0,
+		error = -libxfs_dir_ialloc(&tp, pip, mode|S_IFIFO, 1, 0,
 				&creds, fsxp, &ip);
 		if (error)
 			fail(_("Inode allocation failed"), error);
@@ -537,7 +537,7 @@  parseproto(
 		buf = getstr(pp);
 		len = (int)strlen(buf);
 		tp = getres(mp, XFS_B_TO_FSB(mp, len));
-		error = -libxfs_inode_alloc(&tp, pip, mode|S_IFLNK, 1, 0,
+		error = -libxfs_dir_ialloc(&tp, pip, mode|S_IFLNK, 1, 0,
 				&creds, fsxp, &ip);
 		if (error)
 			fail(_("Inode allocation failed"), error);
@@ -548,7 +548,7 @@  parseproto(
 		break;
 	case IF_DIRECTORY:
 		tp = getres(mp, 0);
-		error = -libxfs_inode_alloc(&tp, pip, mode|S_IFDIR, 1, 0,
+		error = -libxfs_dir_ialloc(&tp, pip, mode|S_IFDIR, 1, 0,
 				&creds, fsxp, &ip);
 		if (error)
 			fail(_("Inode allocation failed"), error);
@@ -640,7 +640,7 @@  rtinit(
 
 	memset(&creds, 0, sizeof(creds));
 	memset(&fsxattrs, 0, sizeof(fsxattrs));
-	error = -libxfs_inode_alloc(&tp, NULL, S_IFREG, 1, 0,
+	error = -libxfs_dir_ialloc(&tp, NULL, S_IFREG, 1, 0,
 					&creds, &fsxattrs, &rbmip);
 	if (error) {
 		fail(_("Realtime bitmap inode allocation failed"), error);
@@ -657,7 +657,7 @@  rtinit(
 	libxfs_trans_log_inode(tp, rbmip, XFS_ILOG_CORE);
 	libxfs_log_sb(tp);
 	mp->m_rbmip = rbmip;
-	error = -libxfs_inode_alloc(&tp, NULL, S_IFREG, 1, 0,
+	error = -libxfs_dir_ialloc(&tp, NULL, S_IFREG, 1, 0,
 					&creds, &fsxattrs, &rsumip);
 	if (error) {
 		fail(_("Realtime summary inode allocation failed"), error);
diff --git a/repair/phase6.c b/repair/phase6.c
index 682356f0..f69afac9 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -919,7 +919,7 @@  mk_orphanage(xfs_mount_t *mp)
 		do_error(_("%d - couldn't iget root inode to make %s\n"),
 			i, ORPHANAGE);*/
 
-	error = -libxfs_inode_alloc(&tp, pip, mode|S_IFDIR,
+	error = -libxfs_dir_ialloc(&tp, pip, mode|S_IFDIR,
 					1, 0, &zerocr, &zerofsx, &ip);
 	if (error) {
 		do_error(_("%s inode allocation failed %d\n"),