Re: [PATCH] xfs_io: Implement inodes64 command

[Dave Chinner <david@fromorbit.com>]

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@redhat.com>
> ---
>  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@fromorbit.com

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