xfs
[Top] [All Lists]

Re: [RFC PATCH] xfs_io: Implement inodes64 command

To: xfs@xxxxxxxxxxx
Subject: Re: [RFC PATCH] xfs_io: Implement inodes64 command
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Thu, 17 Sep 2015 12:02:10 -0500
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1442499846-10470-1-git-send-email-cmaiolino@xxxxxxxxxx>
References: <1442499846-10470-1-git-send-email-cmaiolino@xxxxxxxxxx>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:38.0) Gecko/20100101 Thunderbird/38.2.0
(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.

> 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

> +     xfs_inogrp_t            igroup[64];

why 64?  wouldn't one suffice?

> +     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?

> +     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"

> +
>       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

<Prev in Thread] Current Thread [Next in Thread>