Re: [PATCH] xfs_io: Implement inodes64 command

[Dave Chinner <david@xxxxxxxxxxxxx>]

On Mon, Sep 21, 2015 at 10:56:08AM +0200, Carlos Maiolino wrote:
> Implement an easy way to check a filesystem for the existence of 64bit inodes.
> 
> Based on XFS_IOC_FSINUMBERS, and starting with a lastip of XFS_MAXINUMBER_32, 
> it
> returns a string saying if there is or there isn't 64bit inodes in the
> filesystem.
> 
> 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..b25b09d 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 inodes64_cmd;
>  static prid_t prid;
>  static long extsize;
>  
> @@ -750,6 +752,36 @@ statfs_f(
>       return 0;
>  }
>  
> +static int
> +inodes64_f(
> +       int                   argc,
> +       char                  **argv)
> +{
> +     int                     i;
> +     __s32                   count;
> +     __u64                   last = XFS_MAXINUMBER_32;
> +     xfs_inogrp_t            igroup;
> +     xfs_fsop_bulkreq_t      bulkreq;

Please don't use typedefs.

> +
> +     bulkreq.lastip = &last;
> +     bulkreq.icount = 1;
> +     bulkreq.ubuffer = &igroup;
> +     bulkreq.ocount = &count;
> +
> +     if (!xfsctl(file->name, file->fd, XFS_IOC_FSINUMBERS, &bulkreq)) {
> +             if (count > 0) {
> +                     printf("Filesystem does have 64bit inodes\n");
> +                     return 0;
> +             } else {
> +                     printf("Filesystem does not have 64bit inodes\n");
> +                     return 0;
> +             }
> +     }
> +     perror("xfsctl(XFS_IOC_FSINUMBERS)");

I'd do this the other way around:

        if (xfsctl(file->name, file->fd, XFS_IOC_FSINUMBERS, &bulkreq)) {
                perror("XFS_IOC_FSINUMBERS");
                exitcode = 1;
                return 0;
        }

        if (count > 0)
                printf("Filesystem does have 64bit inodes\n");
        else
                printf("Filesystem does not have 64bit inodes\n");
        return 0;

is sufficient, xfsctl is a one line wrapper around ioctl().

> @@ -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");
                                    ^^^     ^^ 64 bit

"inodes64" could be improved as a command name. e.g:

oneline = _("Query inode number usage in the filesystem")

And could do a little more to help users work out what the
problematic inode is. e.g:

Long help:

Inode_numbers [-s|-l] [num]

        [-s]    returns the physical size of the largest inode
                number in the filesystem (32 bit of 64 bit)
        [-l]    returns the largest inode number in the filesystem
        [-n]    returns the next inode number after [num]
        [num]   returns whether the inode number exists

i.e. if you want 32 bit inodes in the fs, you can use [-n num] to
iterate all the inode numbers above 32 bits in size...

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx