On Thu, Aug 23, 2012 at 04:56:28PM +0800, Liu Bo wrote:
> Add an option 's' to set size in logical to inode transition, then we are able
> to read all the refs to the logical address.
> 
> Signed-off-by: Liu Bo <bo.li....@oracle.com>
> ---
>  cmds-inspect.c |   18 ++++++++++++------
>  1 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/cmds-inspect.c b/cmds-inspect.c
> index 2f0228f..be5e588 100644
> --- a/cmds-inspect.c
> +++ b/cmds-inspect.c
> @@ -115,7 +115,7 @@ static int cmd_inode_resolve(int argc, char **argv)
>  }
>  
>  static const char * const cmd_logical_resolve_usage[] = {
> -     "btrfs inspect-internal logical-resolve [-Pv] <logical> <path>",
> +     "btrfs inspect-internal logical-resolve [-Pv] [-s size] <logical> 
> <path>",

Calling it 'size' si very ambiguous and I needed to go through the code
to see where and how the size is actually used. Something like 'bufsize'
would be more understandable for a regular user.

>       "Get file system paths for the given logical address",
>       NULL

and the option should be documented here, saying that if the list is
incomplete, user can increase it and the maximum value (when we agree on
it)

>  };
> @@ -130,12 +130,13 @@ static int cmd_logical_resolve(int argc, char **argv)
>       int bytes_left;
>       struct btrfs_ioctl_logical_ino_args loi;
>       struct btrfs_data_container *inodes;
> +     u64 size = 4096;
>       char full_path[4096];
>       char *path_ptr;
>  
>       optind = 1;
>       while (1) {
> -             int c = getopt(argc, argv, "Pv");
> +             int c = getopt(argc, argv, "Pvs:");
>               if (c < 0)
>                       break;
>  
> @@ -146,6 +147,9 @@ static int cmd_logical_resolve(int argc, char **argv)
>               case 'v':
>                       verbose = 1;
>                       break;
> +             case 's':
> +                     size = atoll(optarg);
> +                     break;
>               default:
>                       usage(cmd_logical_resolve_usage);
>               }
> @@ -154,12 +158,13 @@ static int cmd_logical_resolve(int argc, char **argv)
>       if (check_argc_exact(argc - optind, 2))
>               usage(cmd_logical_resolve_usage);
>  
> -     inodes = malloc(4096);
> +     size = max(size, 4096);
> +     inodes = malloc(size);

Do we need the sanity checks here as well? Actually, if we avoid the
kernel buffer allocation, then user can supply a buffer of any size here
and we can drop the upper limit. This would be a bit more future proof,
when people will generate not thousands but milions of snapshots :)

>       if (!inodes)
>               return 1;
>  
>       loi.logical = atoll(argv[optind]);
> -     loi.size = 4096;
> +     loi.size = size;
>       loi.inodes = (u64)inodes;
>  
>       fd = open_file_or_dir(argv[optind+1]);
> @@ -176,8 +181,9 @@ static int cmd_logical_resolve(int argc, char **argv)
>       }
>  
>       if (verbose)
> -             printf("ioctl ret=%d, bytes_left=%lu, bytes_missing=%lu, "
> -                     "cnt=%d, missed=%d\n", ret,
> +             printf("ioctl ret=%d, total_size=%llu, bytes_left=%lu, "
> +                     "bytes_missing=%lu, cnt=%d, missed=%d\n",
> +                     ret, size,
>                       (unsigned long)inodes->bytes_left,
>                       (unsigned long)inodes->bytes_missing,
>                       inodes->elem_cnt, inodes->elem_missed);
> -- 
> 1.7.7.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to