On Thu, Sep 17, 2015 at 12:02:10PM -0500, Eric Sandeen wrote:
> (oops, resending to list not just Carlos)
>
> On 9/17/15 9:24 AM, Carlos Maiolino wrote:
> > This command aims to implement an easy way to check a filesystem for the
> > existence of 64bit inodes.
> >
> > Based on XFS_IOC_FSINUMBERS, and starting with a lastip = 0xffffffff
> > instead of 0.
> >
> > For now, it just returns a string saying there is or there is no 64bit inodes
> > in the filesystem, but, I was wondering if it might not be better to just return
> > 0 or 1, so it can be used inside scripts to check for that. Or maybe an argument
> > to chose between an integer output or a string verbose output.
> >
> > Also, I was wondering if might be useful to, besides return the existence of
> > 64bit inodes, also inform if there is any 64bit inodes allocated in the groups
> > or not. Although, this will need the tool to walk through all the inode groups
> > checking for allocated inodes or not, instead of just stop at the first 64bit
> > inode found.
>
> I'm not sure what you mean here - list the groups which contain them?
> Any group above the one where the first 64 bit inode is found will also
> have 64-bit inodes, (unless they have no inodes at all) - so I don't see
> the value, but maybe I'm missing something.
>
Inodes are allocated in 'clusters', you might have a 64-bit inodes cluster
allocated, but not in use at all, the xfs_inogrp_t.xi_allocmask field will show
which inodes from that cluster is allocated or not, so, I was wondering if the
information that "64bit inodes were created" is enough, of if would be useful to
say that '64bits inodes were created and are/aren't in use'.
> > Also, I'm still not sure if 'inodes64' is a good name for the command, I was
> > also thinking about something like 'chk64inos' but 'inodes64' was the best and
> > easy to be remembered I could find.
>
> Eh, seems reasonable to me. Not super, but I can't think of anything much
> better.
>
> > Comments are appreciated
>
> Below...
>
> > Signed-off-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
> > ---
> > io/open.c | 39 +++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 39 insertions(+)
> >
> > diff --git a/io/open.c b/io/open.c
> > index ac5a5e0..3a98fdf 100644
> > --- a/io/open.c
> > +++ b/io/open.c
> > @@ -44,6 +44,7 @@ static cmdinfo_t statfs_cmd;
> > static cmdinfo_t chproj_cmd;
> > static cmdinfo_t lsproj_cmd;
> > static cmdinfo_t extsize_cmd;
> > +static cmdinfo_t inodes64_cmd;
> > static prid_t prid;
> > static long extsize;
> >
> > @@ -750,6 +751,37 @@ statfs_f(
> > return 0;
> > }
> >
> > +static int
> > +inodes64_f(
> > + int argc,
> > + char **argv)
> > +{
> > + int i;
> > + __s32 count;
> > + __u64 last = 0xffffffff;
>
> might be good to use XFS_MAXINUMBER_32 here
>
Good, I didn't know about this flag :)
> > + xfs_inogrp_t igroup[64];
>
> why 64? wouldn't one suffice?
>
well, 64 is the default size of the inode chunks (or clusters, whatever we call
it), so we can get a whole inode cluster at a time.
> > + xfs_fsop_bulkreq_t bulkreq;
> > +
> > + /* Setup bulkreq structure */
> > + bulkreq.lastip = &last;
> > + bulkreq.icount = 64;
> > + bulkreq.ubuffer = &igroup;
> > + bulkreq.ocount = &count;
> > +
> > + while (xfsctl(file->name, file->fd, XFS_IOC_FSINUMBERS, &bulkreq) == 0) {
> > + if (count > 0) {
> > + printf("Filesystem does have 64bit inodes\n");
> > + return 0;
> > + } else {
> > + printf("Filesystem does not have 64bit inodes\n");
> > + return 0;
> > + }
> > + }
>
> I'm also not sure what the "while" is for, here.
>
> If you start at XFS_MAXINUMBER_32, won't a single call either
> give you count = 1 or count = 0?
>
Probably you are right. I used the while() keeping in mind the possibility to
return the status of all 64bit inodes existing in the filesystem, also, I had
this question:
"What if the inode XFS_MAXINUMBER_32 does not exist, but bigger inodes do? Like
in different, bigger AGs?"
I was considering that each call using XFS_IOC_FSINUMBERS, will return only the
inodes in the same allocation group, and another xfsctl call was needed to
continue in the following ones. But I really don't know from where I took it,
probably misinterpreting the xfsctl manpage :)
I should have read the kernel implementation before writing it :)
So, yes, you're right, just a single xfsctl call here will return the next valid
inode bigger than 0xffffffff.
while{} needed only if we want to keep track of the status of the remaining
ones, which, IMHO is not the goal of this command.
> > + perror("xfsctl(XFS_IOC_FSINUMBERS)");
> > + exitcode = 1;
> > + return 0;
> > +}
> > +
> > void
> > open_init(void)
> > {
> > @@ -815,6 +847,12 @@ open_init(void)
> > _("get/set preferred extent size (in bytes) for the open file");
> > extsize_cmd.help = extsize_help;
> >
> > + inodes64_cmd.name = "inodes64";
> > + inodes64_cmd.cfunc = inodes64_f;
> > + inodes64_cmd.flags = CMD_NOMAP_OK;
> > + inodes64_cmd.oneline =
> > + _("Checks if filesyste contais 64bit inodes");
>
> "if filesystem contains"
>
Typo, sorry
> > +
> > add_command(&open_cmd);
> > add_command(&stat_cmd);
> > add_command(&close_cmd);
> > @@ -822,4 +860,5 @@ open_init(void)
> > add_command(&chproj_cmd);
> > add_command(&lsproj_cmd);
> > add_command(&extsize_cmd);
> > + add_command(&inodes64_cmd);
> > }
>
> needs xfs_io manpage updates too, and possibly an xfstest test case?
>
> -Eric
Will do, this was just an RFC to get comments about it. I wasn't willing to
write a manpage entry without even know if people agreed with the command name,
or even with the idea :)
Thanks for the comments, much appreciated.
>
> _______________________________________________
> 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
