[PATCH 0/2] xfs_io: implement inode command

[PATCH 0/2] xfs_io: implement inode command

[Carlos Maiolino]

Implement xfs_io inode command, which can be used to retreive information about
allocated inodes in a xfs filesystem.

Carlos Maiolino (2):
  xfs_io: implement 'inode' command V4
  xfs_io: inode command manpage V1

 io/open.c         | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 man/man8/xfs_io.8 |  24 +++++++++
 2 files changed, 175 insertions(+)

-- 
2.4.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/2] xfs_io: implement 'inode' command V4

[Carlos Maiolino]

Implements a new xfs_io command, named 'inode', which is supposed to be used to
query information about inode's existence and its physical size in the
filesystem.

Currently supporting three arguments:
-s       -- return physical size of the largest inode
-l       -- return the largest inode number allocated and used
-n [num] -- Return the next existing inode after [num], even if [num] is not
            allocated/used
[num]    -- Return if the inode exists or not.

I didn't send the man page patch because I'm sure I'll get some points to
improve, and I'll write the manpage for the next revision.

- Changelog

V3:
	- Merge all 3 patches from the V2 together in a single patch
	- Rework of '-n [num]' and 'num' only arguments algorithm
	- Argument -n now relies on bulkreq.count to check for next inodes, not
	  on bstat.bs_ino anymore.
	- for loop in ret_lsize or ret_largest case, now relies on count being 0
	  to break the loop

V4:
	- Refactor inode_f function to reduce its size and easier logic
	- Implement error handlers for invalid command combination (hopefully
	  all invalid combinations).
	- use a single xfs_inogrp array for keep track of inodes
	- Fix missing newline in inode_help()
	- Rewrite help message in inode_help()
	- Fix indentation

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 io/open.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 151 insertions(+)

diff --git a/io/open.c b/io/open.c
index ac5a5e0..2fc8aab 100644
--- a/io/open.c
+++ b/io/open.c
@@ -20,6 +20,7 @@
 #include "input.h"
 #include "init.h"
 #include "io.h"
+#include "libxfs.h"
 
 #ifndef __O_TMPFILE
 #if defined __alpha__
@@ -44,6 +45,7 @@ static cmdinfo_t statfs_cmd;
 static cmdinfo_t chproj_cmd;
 static cmdinfo_t lsproj_cmd;
 static cmdinfo_t extsize_cmd;
+static cmdinfo_t inode_cmd;
 static prid_t prid;
 static long extsize;
 
@@ -750,6 +752,144 @@ statfs_f(
 	return 0;
 }
 
+static void
+inode_help(void)
+{
+	printf(_(
+"\n"
+"Query physical information about the inode"
+"\n"
+" -l	-- Returns the largest inode number in the filesystem\n"
+" -s	-- Returns the physical size (in bits) of the\n"
+"	   largest inode number in the filesystem\n"
+" -n	-- Return the next valid inode after [num]\n"
+"[num]	   Check if the inode [num] is in use\n"
+"\n"));
+}
+
+static int
+inode_f(
+	  int			argc,
+	  char			**argv)
+{
+	__s32			count = 0;
+	__s32			lastgrp = 0;
+	__u64			last = 0;
+	__u64			lastino = 0;
+	__u64			userino = 0;
+	char			*p;
+	int			c;
+	int			ret_lsize = 0;
+	int			ret_largest = 0;
+	int			ret_next = 0;
+	int			cmd = 0;
+	struct xfs_inogrp	igroup[1024], igrp_rec;
+	struct xfs_fsop_bulkreq	bulkreq;
+	struct xfs_bstat	bstat;
+
+
+	while ((c = getopt(argc, argv, "sln")) != EOF) {
+		switch (c) {
+		case 's':
+			ret_lsize = 1;
+			break;
+		case 'l':
+			ret_largest = 1;
+			break;
+		case 'n':
+			ret_next = 1;
+			break;
+		default:
+			return command_usage(&inode_cmd);
+		}
+	}
+
+	if (optind < argc) {
+		if (ret_lsize || ret_largest)
+			return command_usage(&inode_cmd);
+
+		if (ret_next) {
+			if (argc > 3)
+				return command_usage(&inode_cmd);
+			else
+				cmd = XFS_IOC_FSBULKSTAT;
+		} else {
+			if (argc > 2)
+				return command_usage(&inode_cmd);
+			else
+				cmd = XFS_IOC_FSBULKSTAT_SINGLE;
+		}
+
+
+		userino = strtoull(argv[optind], &p, 10);
+		if ((*p != '\0')) {
+			printf(_("[num] must be a numeric value\n"));
+			exitcode = 1;
+			return 0;
+		}
+
+		bulkreq.lastip = &userino;
+		bulkreq.icount = 1;
+		bulkreq.ubuffer = &bstat;
+		bulkreq.ocount = &count;
+
+		if (xfsctl(file->name, file->fd, cmd, &bulkreq)) {
+			if (errno == EINVAL)
+				printf(_("Invalid or unlinked inode\n"));
+			else
+				perror("xfsctl");
+			exitcode = 1;
+			return 0;
+		}
+
+		if (ret_next) {
+			printf(_("Next inode: %llu\n"), bstat.bs_ino);
+			return 0;
+		} else {
+			printf(_("Valid inode: %llu\n"), bstat.bs_ino);
+			return 0;
+		}
+	}
+
+	if (ret_lsize || ret_largest) {
+
+		bulkreq.lastip = &last;
+		bulkreq.icount = 1024; /* User-defined maybe!? */
+		bulkreq.ubuffer = &igroup;
+		bulkreq.ocount = &count;
+
+		for (;;) {
+
+			if (xfsctl(file->name, file->fd, XFS_IOC_FSINUMBERS,
+					&bulkreq)) {
+				perror("XFS_IOC_FSINUMBERS");
+				exitcode = 1;
+				return 0;
+			}
+
+			if (count == 0)
+				break;
+
+			lastgrp = count;
+		}
+
+		lastgrp--;
+		igrp_rec = igroup[lastgrp];
+		lastino = igrp_rec.xi_startino +
+			  xfs_highbit64(igrp_rec.xi_allocmask);
+
+		if (ret_lsize)
+			printf (_("Largest inode size: %d\n"),
+				lastino > XFS_MAXINUMBER_32 ? 64 : 32);
+		else
+			printf(_("Largest inode: %llu\n"), lastino);
+
+		return 0;
+	}
+
+	return command_usage(&inode_cmd);
+}
+
 void
 open_init(void)
 {
@@ -815,6 +955,16 @@ open_init(void)
 		_("get/set preferred extent size (in bytes) for the open file");
 	extsize_cmd.help = extsize_help;
 
+	inode_cmd.name = "inode";
+	inode_cmd.cfunc = inode_f;
+	inode_cmd.args = _("[-s | -l | -n] [num]");
+	inode_cmd.argmin = 1;
+	inode_cmd.argmax = 2;
+	inode_cmd.flags = CMD_NOMAP_OK;
+	inode_cmd.oneline =
+		_("Query inode number usage in the filesystem");
+	inode_cmd.help = inode_help;
+
 	add_command(&open_cmd);
 	add_command(&stat_cmd);
 	add_command(&close_cmd);
@@ -822,4 +972,5 @@ open_init(void)
 	add_command(&chproj_cmd);
 	add_command(&lsproj_cmd);
 	add_command(&extsize_cmd);
+	add_command(&inode_cmd);
 }
-- 
2.4.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/2] xfs_io: inode command manpage V1

[Carlos Maiolino]

This is the first try to update xfs_io manpage to include information about
'inode' command usage.

Improvements and grammar corrections are welcome :-)

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 man/man8/xfs_io.8 | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 416206f..457f0eb 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -490,7 +490,31 @@ Recursively display all the specified segments starting at the specified
 .B \-s
 Display the starting lseek(2) offset. This offset will be a calculated value when
 both data and holes are displayed together or performing a recusively display.
+.RE
+.PD
+.TP
+.TP
+.BI "inode  \-s | \-l | \-n [num] | [num]"
+Query physical inode information from a xfs filesystem.
+.RS 1.0i
+.PD 0
+.TP 0.4i
+.B \-s
+Display the physical size of the largest inode existing in the filesystem (32 or
+64 bit).
+.TP
+.B -l
+Display the largest allocated and used inode (inodes can be allocated in the
+filesystem, but not used by any file). See
+.B ikeep
+xfs mount option).
+.TP
+.B -n [num]
+Return the next existing inode after [num], even if [num] is not allocated in
+the filesystem.
 .TP
+.B [num]
+Check if the inode [num] exists or not.
 
 .SH MEMORY MAPPED I/O COMMANDS
 .TP
-- 
2.4.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] xfs_io: implement 'inode' command V4

[Brian Foster]

On Mon, Nov 16, 2015 at 09:43:23AM +0100, Carlos Maiolino wrote:
> Implements a new xfs_io command, named 'inode', which is supposed to be used to
> query information about inode's existence and its physical size in the
> filesystem.
> 
> Currently supporting three arguments:
> -s       -- return physical size of the largest inode
> -l       -- return the largest inode number allocated and used
> -n [num] -- Return the next existing inode after [num], even if [num] is not
>             allocated/used
> [num]    -- Return if the inode exists or not.
> 
> I didn't send the man page patch because I'm sure I'll get some points to
> improve, and I'll write the manpage for the next revision.
> 
> - Changelog
> 
> V3:
> 	- Merge all 3 patches from the V2 together in a single patch
> 	- Rework of '-n [num]' and 'num' only arguments algorithm
> 	- Argument -n now relies on bulkreq.count to check for next inodes, not
> 	  on bstat.bs_ino anymore.
> 	- for loop in ret_lsize or ret_largest case, now relies on count being 0
> 	  to break the loop
> 
> V4:
> 	- Refactor inode_f function to reduce its size and easier logic
> 	- Implement error handlers for invalid command combination (hopefully
> 	  all invalid combinations).
> 	- use a single xfs_inogrp array for keep track of inodes
> 	- Fix missing newline in inode_help()
> 	- Rewrite help message in inode_help()
> 	- Fix indentation
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---

Well, the code looks Ok to me but the design still seems overdone IMO. I
have no major objection if this goes in as is (with one exception noted
below), but IMO we should take the approach somewhat discussed in the v3
review.

In particular, define an actual default behavior for the command to
return the largest inode number (or return 1/0 for "ability to mount w/
inode32" as Dave suggested). For example, kill both of the -l and -s
flags and just return 1/0 by default. Define a single verbose (-v) flag
to print the combined inode number and size. This mode can be
implemented as the body of the inode_f() function. If -n is specified,
basically do what the current version does. Just my .02.

>  io/open.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 151 insertions(+)
> 
> diff --git a/io/open.c b/io/open.c
> index ac5a5e0..2fc8aab 100644
> --- a/io/open.c
> +++ b/io/open.c
...
> +	if (ret_lsize || ret_largest) {
> +
> +		bulkreq.lastip = &last;
> +		bulkreq.icount = 1024; /* User-defined maybe!? */
> +		bulkreq.ubuffer = &igroup;
> +		bulkreq.ocount = &count;
> +
> +		for (;;) {
> +
> +			if (xfsctl(file->name, file->fd, XFS_IOC_FSINUMBERS,
> +					&bulkreq)) {
> +				perror("XFS_IOC_FSINUMBERS");
> +				exitcode = 1;
> +				return 0;
> +			}
> +
> +			if (count == 0)
> +				break;
> +
> +			lastgrp = count;
> +		}
> +
> +		lastgrp--;
> +		igrp_rec = igroup[lastgrp];

IIRC the point of igrp_rec was to save off the last record so it could
be used directly after the loop without need for the count (because it
could be 0). Here we use a separate lastgrp count to protect against the
0 case, yet still do the record copy after the loop... what's the point
of that?

Brian

> +		lastino = igrp_rec.xi_startino +
> +			  xfs_highbit64(igrp_rec.xi_allocmask);
> +
> +		if (ret_lsize)
> +			printf (_("Largest inode size: %d\n"),
> +				lastino > XFS_MAXINUMBER_32 ? 64 : 32);
> +		else
> +			printf(_("Largest inode: %llu\n"), lastino);
> +
> +		return 0;
> +	}
> +
> +	return command_usage(&inode_cmd);
> +}
> +
>  void
>  open_init(void)
>  {
> @@ -815,6 +955,16 @@ open_init(void)
>  		_("get/set preferred extent size (in bytes) for the open file");
>  	extsize_cmd.help = extsize_help;
>  
> +	inode_cmd.name = "inode";
> +	inode_cmd.cfunc = inode_f;
> +	inode_cmd.args = _("[-s | -l | -n] [num]");
> +	inode_cmd.argmin = 1;
> +	inode_cmd.argmax = 2;
> +	inode_cmd.flags = CMD_NOMAP_OK;
> +	inode_cmd.oneline =
> +		_("Query inode number usage in the filesystem");
> +	inode_cmd.help = inode_help;
> +
>  	add_command(&open_cmd);
>  	add_command(&stat_cmd);
>  	add_command(&close_cmd);
> @@ -822,4 +972,5 @@ open_init(void)
>  	add_command(&chproj_cmd);
>  	add_command(&lsproj_cmd);
>  	add_command(&extsize_cmd);
> +	add_command(&inode_cmd);
>  }
> -- 
> 2.4.3
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] xfs_io: implement 'inode' command V4

[Carlos Maiolino]

Hi Brian,

> 
> Well, the code looks Ok to me but the design still seems overdone IMO. I
> have no major objection if this goes in as is (with one exception noted
> below), but IMO we should take the approach somewhat discussed in the v3
> review.

First of all, thanks for reviewing the patch again, I took some time to reply
because I was waiting for any other comments and had a few other things to do.

> 
> In particular, define an actual default behavior for the command to
> return the largest inode number (or return 1/0 for "ability to mount w/
> inode32" as Dave suggested). For example, kill both of the -l and -s
> flags and just return 1/0 by default. Define a single verbose (-v) flag
> to print the combined inode number and size. This mode can be
> implemented as the body of the inode_f() function. If -n is specified,
> basically do what the current version does. Just my .02.
> 

I agree with you here and tbh, I don't really think a -v flag is really needed,
although it can certainly facilitate the usage of the xfs_io -c "inode".
Checking for the size of the inode returned against UINT32_MAX is not that hard
anyway, but I think keeping a very simple return value for the command might be
the best approach.

I'm going to re-write it and send a V5 today including a review of your comment
below.

thanks again for the review

> >  io/open.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 151 insertions(+)
> > 
> > diff --git a/io/open.c b/io/open.c
> > index ac5a5e0..2fc8aab 100644
> > --- a/io/open.c
> > +++ b/io/open.c
> ...
> > +	if (ret_lsize || ret_largest) {
> > +
> > +		bulkreq.lastip = &last;
> > +		bulkreq.icount = 1024; /* User-defined maybe!? */
> > +		bulkreq.ubuffer = &igroup;
> > +		bulkreq.ocount = &count;
> > +
> > +		for (;;) {
> > +
> > +			if (xfsctl(file->name, file->fd, XFS_IOC_FSINUMBERS,
> > +					&bulkreq)) {
> > +				perror("XFS_IOC_FSINUMBERS");
> > +				exitcode = 1;
> > +				return 0;
> > +			}
> > +
> > +			if (count == 0)
> > +				break;
> > +
> > +			lastgrp = count;
> > +		}
> > +
> > +		lastgrp--;
> > +		igrp_rec = igroup[lastgrp];
> 
> IIRC the point of igrp_rec was to save off the last record so it could
> be used directly after the loop without need for the count (because it
> could be 0). Here we use a separate lastgrp count to protect against the
> 0 case, yet still do the record copy after the loop... what's the point
> of that?
> 
> Brian
> 
> > +		lastino = igrp_rec.xi_startino +
> > +			  xfs_highbit64(igrp_rec.xi_allocmask);
> > +
> > +		if (ret_lsize)
> > +			printf (_("Largest inode size: %d\n"),
> > +				lastino > XFS_MAXINUMBER_32 ? 64 : 32);
> > +		else
> > +			printf(_("Largest inode: %llu\n"), lastino);
> > +
> > +		return 0;
> > +	}
> > +
> > +	return command_usage(&inode_cmd);
> > +}
> > +
> >  void
> >  open_init(void)
> >  {
> > @@ -815,6 +955,16 @@ open_init(void)
> >  		_("get/set preferred extent size (in bytes) for the open file");
> >  	extsize_cmd.help = extsize_help;
> >  
> > +	inode_cmd.name = "inode";
> > +	inode_cmd.cfunc = inode_f;
> > +	inode_cmd.args = _("[-s | -l | -n] [num]");
> > +	inode_cmd.argmin = 1;
> > +	inode_cmd.argmax = 2;
> > +	inode_cmd.flags = CMD_NOMAP_OK;
> > +	inode_cmd.oneline =
> > +		_("Query inode number usage in the filesystem");
> > +	inode_cmd.help = inode_help;
> > +
> >  	add_command(&open_cmd);
> >  	add_command(&stat_cmd);
> >  	add_command(&close_cmd);
> > @@ -822,4 +972,5 @@ open_init(void)
> >  	add_command(&chproj_cmd);
> >  	add_command(&lsproj_cmd);
> >  	add_command(&extsize_cmd);
> > +	add_command(&inode_cmd);
> >  }
> > -- 
> > 2.4.3
> > 
> > _______________________________________________
> > xfs mailing list
> > xfs@oss.sgi.com
> > http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs