* [PATCH 0/5] xfs_io: address various "inode" command issues
@ 2016-09-30 20:50 Eric Sandeen
2016-09-30 20:52 ` [PATCH 1/5] xfs_io: fix inode command help and argsmax Eric Sandeen
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Eric Sandeen @ 2016-09-30 20:50 UTC (permalink / raw)
To: linux-xfs
With these 5 patches the fstest I sent yesterday will
pass.
A lot of it is code re-org, but there a couple small bugs
resolved as well.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/5] xfs_io: fix inode command help and argsmax
2016-09-30 20:50 [PATCH 0/5] xfs_io: address various "inode" command issues Eric Sandeen
@ 2016-09-30 20:52 ` Eric Sandeen
2016-09-30 20:53 ` [PATCH 2/5] xfs_io: factor out new get_last_inode() helper Eric Sandeen
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Eric Sandeen @ 2016-09-30 20:52 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs
The short help implied that -n and -v were exclusive, and the longer
help wasn't particularly clear.
Further, argsmax is wrong; "-n -v num" is 3, not 2.
# xfs_io -c "inode -n -v 123" /mnt/test2
bad argument count 3 to inode, expected between 0 and 2 arguments
# xfs_io -c "inode -vn 123" /mnt/test2
128:32
Fix up all of those issues.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
io/open.c | 19 ++++++++-----------
1 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/io/open.c b/io/open.c
index 3eaa013..5efa739 100644
--- a/io/open.c
+++ b/io/open.c
@@ -757,16 +757,13 @@ inode_help(void)
{
printf(_(
"\n"
-"Query physical information about the inode"
+"Query physical information about an inode"
"\n"
-" Default: -- Return true(1) or false(0) if any inode greater than\n"
-" 32bits has been found in the filesystem\n"
-"[num] -- Return inode number [num] or 0 if the inode [num] is in use\n"
-" or not\n"
-" -n [num] -- Return the next valid inode after [num]\n"
-" -v -- verbose mode\n"
-" Display the inode number and its physical size (in bits)\n"
-" according to the argument used\n"
+" Default: -- Return 1 if any inode number greater than 32 bits exists in\n"
+" the filesystem, or 0 if none exist\n"
+" num -- Return inode number [num] if in use, or 0 if not in use\n"
+" -n num -- Return the next used inode after [num]\n"
+" -v -- Verbose mode - display returned inode number's size in bits\n"
"\n"));
}
@@ -956,9 +953,9 @@ open_init(void)
inode_cmd.name = "inode";
inode_cmd.cfunc = inode_f;
- inode_cmd.args = _("[-n | -v] [num]");
+ inode_cmd.args = _("[-nv] [num]");
inode_cmd.argmin = 0;
- inode_cmd.argmax = 2;
+ inode_cmd.argmax = 3;
inode_cmd.flags = CMD_NOMAP_OK;
inode_cmd.oneline =
_("Query inode number usage in the filesystem");
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/5] xfs_io: factor out new get_last_inode() helper
2016-09-30 20:50 [PATCH 0/5] xfs_io: address various "inode" command issues Eric Sandeen
2016-09-30 20:52 ` [PATCH 1/5] xfs_io: fix inode command help and argsmax Eric Sandeen
@ 2016-09-30 20:53 ` Eric Sandeen
2016-09-30 20:54 ` [PATCH 3/5] xfs_io: move inode command arg handling to top Eric Sandeen
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Eric Sandeen @ 2016-09-30 20:53 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs
The inode command by default finds the last allocated
inode in the filesystem via bulkstat, and this specific
function is open-coded after other cases are handled, leading
to a fairly long inode_f function and confusing code flow.
Clean it up by factoring it into a new function, more refactoring
will follow.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
io/open.c | 68 +++++++++++++++++++++++++++++++++++++++---------------------
1 files changed, 44 insertions(+), 24 deletions(-)
diff --git a/io/open.c b/io/open.c
index 5efa739..c8c82b7 100644
--- a/io/open.c
+++ b/io/open.c
@@ -767,14 +767,51 @@ inode_help(void)
"\n"));
}
+static __u64
+get_last_inode(void)
+{
+ __u64 lastip = 0;
+ __u64 lastgrp = 0;
+ __s32 ocount = 0;
+ __u64 last_ino;
+ struct xfs_inogrp igroup[1024];
+ struct xfs_fsop_bulkreq bulkreq;
+
+ bulkreq.lastip = &lastip;
+ bulkreq.ubuffer = &igroup;
+ bulkreq.icount = sizeof(igroup) / sizeof(struct xfs_inogrp);
+ bulkreq.ocount = &ocount;
+
+ for (;;) {
+ if (xfsctl(file->name, file->fd, XFS_IOC_FSINUMBERS,
+ &bulkreq)) {
+ perror("XFS_IOC_FSINUMBERS");
+ return 0;
+ }
+
+ /* Did we reach the last inode? */
+ if (ocount == 0)
+ break;
+
+ /* last inode in igroup table */
+ lastgrp = ocount;
+ }
+
+ lastgrp--;
+
+ /* The last inode number in use */
+ last_ino = igroup[lastgrp].xi_startino +
+ libxfs_highbit64(igroup[lastgrp].xi_allocmask);
+
+ return last_ino;
+}
+
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;
@@ -782,7 +819,6 @@ inode_f(
int verbose = 0;
int ret_next = 0;
int cmd = 0;
- struct xfs_inogrp igroup[1024];
struct xfs_fsop_bulkreq bulkreq;
struct xfs_bstat bstat;
@@ -854,29 +890,13 @@ inode_f(
return command_usage(&inode_cmd);
}
- 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;
+ /* We are finding last inode in use */
+ lastino = get_last_inode();
+ if (!lastino) {
+ exitcode = 1;
+ return 0;
}
- lastgrp--;
- lastino = igroup[lastgrp].xi_startino +
- libxfs_highbit64(igroup[lastgrp].xi_allocmask);
-
if (verbose)
printf("%llu:%d\n", lastino,
lastino > XFS_MAXINUMBER_32 ? 64 : 32);
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/5] xfs_io: move inode command arg handling to top
2016-09-30 20:50 [PATCH 0/5] xfs_io: address various "inode" command issues Eric Sandeen
2016-09-30 20:52 ` [PATCH 1/5] xfs_io: fix inode command help and argsmax Eric Sandeen
2016-09-30 20:53 ` [PATCH 2/5] xfs_io: factor out new get_last_inode() helper Eric Sandeen
@ 2016-09-30 20:54 ` Eric Sandeen
2016-09-30 20:56 ` [PATCH 4/5] xfs_io: refactor inode command Eric Sandeen
2016-09-30 20:57 ` [PATCH 5/5] xfs_io: fix inode command with "-n" for bogus inode Eric Sandeen
4 siblings, 0 replies; 6+ messages in thread
From: Eric Sandeen @ 2016-09-30 20:54 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs
As it stands, collecting the inode number and testing args
validity is all tangled up; for example the test for
"-n" having no inode is buried in an else after a large
code block which handles something else.
Get inode number argument collection and testing out of the
way before doing anything else.
Clean up the error message if a non-numeric inode arg is given.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
io/open.c | 40 +++++++++++++++++++++-------------------
1 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/io/open.c b/io/open.c
index c8c82b7..ba8b243 100644
--- a/io/open.c
+++ b/io/open.c
@@ -813,7 +813,7 @@ inode_f(
{
__s32 count = 0;
__u64 lastino = 0;
- __u64 userino = 0;
+ __u64 userino = NULLFSINO;
char *p;
int c;
int verbose = 0;
@@ -835,27 +835,32 @@ inode_f(
}
}
- /*
- * Inode number can be passed with or without extra arguments, so we
- * should handle inode numbers passed by user out of getopt()
- */
+ /* Last arg (if present) should be an inode number */
if (optind < argc) {
-
- if (ret_next) {
- cmd = XFS_IOC_FSBULKSTAT;
- } else {
- if ((argc > 2) && !verbose)
- 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"));
+ printf(_("%s is not a numeric inode value\n"),
+ argv[optind]);
exitcode = 1;
return 0;
}
+ optind++;
+ }
+
+ /* Extra junk? */
+ if (optind < argc)
+ return command_usage(&inode_cmd);
+
+ /* -n option requires an inode number */
+ if (ret_next && userino == NULLFSINO)
+ return command_usage(&inode_cmd);
+
+ if (userino != NULLFSINO) {
+
+ if (ret_next) /* get next inode */
+ cmd = XFS_IOC_FSBULKSTAT;
+ else /* get this inode */
+ cmd = XFS_IOC_FSBULKSTAT_SINGLE;
bulkreq.lastip = &userino;
bulkreq.icount = 1;
@@ -885,9 +890,6 @@ inode_f(
printf("%llu\n", userino);
return 0;
- /* -n option must not be used stand alone */
- } else if (ret_next) {
- return command_usage(&inode_cmd);
}
/* We are finding last inode in use */
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/5] xfs_io: refactor inode command
2016-09-30 20:50 [PATCH 0/5] xfs_io: address various "inode" command issues Eric Sandeen
` (2 preceding siblings ...)
2016-09-30 20:54 ` [PATCH 3/5] xfs_io: move inode command arg handling to top Eric Sandeen
@ 2016-09-30 20:56 ` Eric Sandeen
2016-09-30 20:57 ` [PATCH 5/5] xfs_io: fix inode command with "-n" for bogus inode Eric Sandeen
4 siblings, 0 replies; 6+ messages in thread
From: Eric Sandeen @ 2016-09-30 20:56 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs
The inode_f function is a bit convoluted; the default
find-last-inode case appears at the end, there are several return
points, we print the same basic information using 2 different variables
in 2 different locations depending on the mode we're in, the
"inode not found" was a printf & exit in the middle of the function,
etc.
Move the default case up to the top so it's more obvious, not
buried.
Make a new var, result_ino, which holds whatever we want to print
regardless of the mode, and then handle all the output at the end.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
io/open.c | 65 ++++++++++++++++++++++++++++--------------------------------
1 files changed, 30 insertions(+), 35 deletions(-)
diff --git a/io/open.c b/io/open.c
index ba8b243..7d952a0 100644
--- a/io/open.c
+++ b/io/open.c
@@ -812,7 +812,7 @@ inode_f(
char **argv)
{
__s32 count = 0;
- __u64 lastino = 0;
+ __u64 result_ino = 0;
__u64 userino = NULLFSINO;
char *p;
int c;
@@ -855,8 +855,14 @@ inode_f(
if (ret_next && userino == NULLFSINO)
return command_usage(&inode_cmd);
- if (userino != NULLFSINO) {
-
+ if (userino == NULLFSINO) {
+ /* We are finding last inode in use */
+ result_ino = get_last_inode();
+ if (!result_ino) {
+ exitcode = 1;
+ return 0;
+ }
+ } else {
if (ret_next) /* get next inode */
cmd = XFS_IOC_FSBULKSTAT;
else /* get this inode */
@@ -868,43 +874,32 @@ inode_f(
bulkreq.ocount = &count;
if (xfsctl(file->name, file->fd, cmd, &bulkreq)) {
- if (errno == EINVAL) {
- if (!ret_next)
- printf("0\n");
+ if (!ret_next && errno == EINVAL) {
+ /* Not in use */
+ result_ino = 0;
} else {
perror("xfsctl");
+ exitcode = 1;
+ return 0;
}
- exitcode = 1;
- return 0;
- }
-
- if (ret_next)
- userino = bstat.bs_ino;
-
- if (verbose)
- printf("%llu:%d\n",
- userino,
- userino > XFS_MAXINUMBER_32 ? 64 : 32);
- else
- /* Inode in use */
- printf("%llu\n", userino);
- return 0;
-
- }
-
- /* We are finding last inode in use */
- lastino = get_last_inode();
- if (!lastino) {
- exitcode = 1;
- return 0;
+ } else if (ret_next) /* The next inode in use */
+ result_ino = bstat.bs_ino;
+ else /* The inode we asked about */
+ result_ino = userino;
+ }
+
+ if (verbose && result_ino) {
+ /* Requested verbose and we have an answer */
+ printf("%llu:%d\n", result_ino,
+ result_ino > XFS_MAXINUMBER_32 ? 64 : 32);
+ } else if (userino == NULLFSINO) {
+ /* Just checking 32 or 64 bit presence, non-verbose */
+ printf("%d\n", result_ino > XFS_MAXINUMBER_32 ? 1 : 0);
+ } else {
+ /* We asked about a specific inode, non-verbose */
+ printf("%llu\n", result_ino);
}
- if (verbose)
- printf("%llu:%d\n", lastino,
- lastino > XFS_MAXINUMBER_32 ? 64 : 32);
- else
- printf("%d\n", lastino > XFS_MAXINUMBER_32 ? 1 : 0);
-
return 0;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 5/5] xfs_io: fix inode command with "-n" for bogus inode
2016-09-30 20:50 [PATCH 0/5] xfs_io: address various "inode" command issues Eric Sandeen
` (3 preceding siblings ...)
2016-09-30 20:56 ` [PATCH 4/5] xfs_io: refactor inode command Eric Sandeen
@ 2016-09-30 20:57 ` Eric Sandeen
4 siblings, 0 replies; 6+ messages in thread
From: Eric Sandeen @ 2016-09-30 20:57 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs
If we ask for the next allocated inode after a number for which
no other inode exists, the bulkstat returns success, but with
count == 0. If we ignore this fact, we print a garbage result
from bstat.bs_ino in this case, so fix it.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
io/open.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/io/open.c b/io/open.c
index 7d952a0..4279062 100644
--- a/io/open.c
+++ b/io/open.c
@@ -882,10 +882,16 @@ inode_f(
exitcode = 1;
return 0;
}
- } else if (ret_next) /* The next inode in use */
- result_ino = bstat.bs_ino;
- else /* The inode we asked about */
+ } else if (ret_next) {
+ /* The next inode in use, or 0 if none */
+ if (*bulkreq.ocount)
+ result_ino = bstat.bs_ino;
+ else
+ result_ino = 0;
+ } else {
+ /* The inode we asked about */
result_ino = userino;
+ }
}
if (verbose && result_ino) {
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-30 20:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-30 20:50 [PATCH 0/5] xfs_io: address various "inode" command issues Eric Sandeen
2016-09-30 20:52 ` [PATCH 1/5] xfs_io: fix inode command help and argsmax Eric Sandeen
2016-09-30 20:53 ` [PATCH 2/5] xfs_io: factor out new get_last_inode() helper Eric Sandeen
2016-09-30 20:54 ` [PATCH 3/5] xfs_io: move inode command arg handling to top Eric Sandeen
2016-09-30 20:56 ` [PATCH 4/5] xfs_io: refactor inode command Eric Sandeen
2016-09-30 20:57 ` [PATCH 5/5] xfs_io: fix inode command with "-n" for bogus inode Eric Sandeen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.