On Tue, Oct 06, 2015 at 01:00:56PM -0400, Brian Foster wrote:
> On Fri, Sep 25, 2015 at 03:07:47PM +0200, Carlos Maiolino wrote:
> > "-n [num]" argument, will return to the user the next inode valid on the filesystem
> > after [num].
> >
> > Using [num] exclusive, will test if the inode [num] is a valid inode in the
> > filesystem or not.
> >
> > Signed-off-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
> > ---
> > io/open.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--------
> > 1 file changed, 74 insertions(+), 10 deletions(-)
> >
> > diff --git a/io/open.c b/io/open.c
> > index 57ff0bf..9f68de0 100644
> > --- a/io/open.c
> > +++ b/io/open.c
> > @@ -762,6 +762,8 @@ inode_help(void)
> > " -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]"
> > +"[num] Check if the inode [num] exists in the filesystem"
> > "\n"));
> > }
> >
> > @@ -774,18 +776,19 @@ inode_f(
> > __s32 lastgrp = 0;
> > __u64 last = 0;
> > __u64 lastino = 0;
> > - struct xfs_inogrp igroup[1024];
> > - struct xfs_fsop_bulkreq bulkreq;
> > + __u64 userino = 0;
> > + char *p;
> > int c;
> > int ret_lsize = 0;
> > int ret_largest = 0;
> > + int ret_isvalid = 0;
> > + int ret_next = 0;
> > + struct xfs_inogrp igroup[1024];
> > + struct xfs_fsop_bulkreq bulkreq;
> > + struct xfs_bstat bstat;
> >
> > - bulkreq.lastip = &last;
> > - bulkreq.icount = 1024; /* maybe an user-defined value!? */
> > - bulkreq.ubuffer = &igroup;
> > - bulkreq.ocount = &count;
> >
> > - while ((c = getopt(argc, argv, "sl")) != EOF) {
> > + while ((c = getopt(argc, argv, "sln:")) != EOF) {
> > switch (c) {
> > case 's':
> > ret_lsize = 1;
> > @@ -793,12 +796,34 @@ inode_f(
> > case 'l':
> > ret_largest = 1;
> > break;
> > + case 'n':
> > + ret_next = 1;
> > + userino = strtoull(optarg, &p, 10);
> > + break;
> > default:
> > return command_usage(&inode_cmd);
> > }
> > }
> >
> > + if ((optind < argc) && !(ret_next || ret_lsize || ret_largest)) {
> > + ret_isvalid = 1;
> > + userino = strtoull(argv[optind], &p, 10);
> > + }
>
> So this appears to be the default behavior (validate whether an inode
> exists). Perhaps this functionality should come first since that's the
> core behavior for the command.
>
Hmm, I don't think so, I need getopt() to setup optind for this.
> > +
> > + if (userino)
> > + if (*p != '\0') {
> > + printf("[num] must be a valid number\n");
> > + exitcode = 1;
> > + 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)) {
> > @@ -806,7 +831,7 @@ inode_f(
> > exitcode = 1;
> > return 0;
> > }
> > - if (count < XFS_INODES_PER_CHUNK && count > 0)
> > + if (count < 1024 && count > 0)
> > lastgrp = count;
>
> Ok, that sort of addresses my question on patch 1. I guess this is a
> record count rather than an inode count as well. In that case, what
> happens if the fs has an exact multiple of 1024 inode records?
>
Yes, it's a record count, each record contains a single inode chunk.
regarding the exactly multiple of 1024 chunks, AFAIK it will fit all the 1024
records in a single array, which is exactly the size of the array I'm using
here, and, next call to xfsctl, will return a 0 records count, making the look
to exit.
> BTW, I think this should probably be set correctly when it is introduced
> rather than set to a value and changed in a subsequent patch.
Yes, I just forgot to change this in the first patch, see my comment in patch 1.
>
> > if (!count)
> > break;
> > @@ -822,8 +847,47 @@ inode_f(
> > else
> > printf(_("Largest inode: %llu\n"), lastino);
> >
> > + return 0;
> > + }
> > +
> > + /* Setup bulkreq for -n or [num] only */
> > + last = userino;
> > + bulkreq.lastip = &last;
> > + bulkreq.icount = 1;
> > + bulkreq.ubuffer = &bstat;
> > + bulkreq.ocount = &count;
> > +
> > + if (ret_next) {
> > + if (xfsctl(file->name, file->fd, XFS_IOC_FSBULKSTAT, &bulkreq)) {
> > + if (errno == EINVAL)
> > + printf("Invalid or non-existent inode\n");
> > + else
> > + perror("XFS_IOC_FSBULKSTAT");
> > + exitcode = 1;
> > + return 0;
> > + }
> > +
> > + if (!bstat.bs_ino) {
> > + printf("There are no further inodes in the filesystem\n");
> > + return 0;
> > + }
>
> The above should technically check the output count rather than the
> inode number, right?
>
If I use the inode count, I can get an 'allocated but free' inode, which is not
the intention here.
> > +
> > + printf("Next inode: %llu\n", bstat.bs_ino);
> > + return 0;
> > }
> >
> > + if (ret_isvalid) {
> > + if (xfsctl(file->name, file->fd, XFS_IOC_FSBULKSTAT_SINGLE, &bulkreq)) {
> > + if (errno == EINVAL) {
> > + printf("Invalid or non-existent inode number\n");
>
> Is EINVAL returned in the non-existent case or ENOENT?
>
I'll check this inside kernel, but I'm not sure if there is any distinction. If
the inode passed to xfsctl is incalid, EINVAL will be returned.
> Brian
>
> > + } else
> > + perror("XFS_IOC_FSBULKSTAT_SINGLE");
> > + exitcode = 1;
> > + return 0;
> > + }
> > + printf("Valid inode: %llu\n", bstat.bs_ino);
> > + return 0;
> > + }
> >
> > return 0;
> > }
> > @@ -895,9 +959,9 @@ open_init(void)
> >
> > inode_cmd.name = "inode";
> > inode_cmd.cfunc = inode_f;
> > - inode_cmd.args = _("[-s | -l]");
> > + inode_cmd.args = _("[-s | -l | -n] [num]");
> > inode_cmd.argmin = 1;
> > - inode_cmd.argmax = 1;
> > + inode_cmd.argmax = 2;
> > inode_cmd.flags = CMD_NOMAP_OK;
> > inode_cmd.oneline =
> > _("Query inode number usage in the filesystem");
> > --
> > 2.4.3
> >
> > _______________________________________________
> > xfs mailing list
> > xfs@xxxxxxxxxxx
> > http://oss.sgi.com/mailman/listinfo/xfs
--
Carlos
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
