Re: [PATCH] Btrfs-progs: add dedup register

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, May 02, 2013 at 12:27:39AM +0800, Liu Bo wrote:
> +static int cmd_dedup(int argc, char **argv)
> +{
> +	int 	fd, res, e;
> +	char	*path;
> +
> +	if (check_argc_exact(argc, 2))
> +		usage(cmd_dedup_usage);
> +
> +	path = argv[1];
> +
> +	fd = open_file_or_dir(path);
> +	if (fd < 0) {
> +		fprintf(stderr, "ERROR: can't access to '%s'\n", path);
> +		return 12;

Please do not introduce the strange return values in new code.

> +	}
> +
> +	printf("register dedup on '%s'\n", path);
> +	res = ioctl(fd, BTRFS_IOC_DEDUP_REGISTER);
> +	e = errno;
> +	close(fd);
> +	if( res < 0 ){
> +		fprintf(stderr, "ERROR: unable to register dedup '%s' - %s\n", 
> +			path, strerror(e));
> +		return 32;

dtto

> +	}
> +
> +	return 0;
> +}
> +
>  const struct cmd_group filesystem_cmd_group = {
>  	filesystem_cmd_group_usage, NULL, {
>  		{ "df", cmd_df, cmd_df_usage, NULL, 0 },
> @@ -524,6 +559,7 @@ const struct cmd_group filesystem_cmd_group = {
>  		{ "balance", cmd_balance, NULL, &balance_cmd_group, 1 },
>  		{ "resize", cmd_resize, cmd_resize_usage, NULL, 0 },
>  		{ "label", cmd_label, cmd_label_usage, NULL, 0 },
> +		{ "dedup-register", cmd_dedup, cmd_dedup_usage, NULL, 0 },

By this you effectively capture the 'dedup*' namespace by a single
command. I thinkg dedup is a bigger beast and should use subcommands
rather than bunch of 1st level subcommands, ie. something like balance
or scrub.

If you don't want to spend time on polishing the UI of progs right now,
fine, just add the commands as you like, this can be fixed later. With a
warning.

>  		{ 0, 0, 0, 0, 0 },
>  	}
>  };
> diff --git a/ioctl.h b/ioctl.h
> index e841913..1bea98a 100644
> --- a/ioctl.h
> +++ b/ioctl.h
> @@ -528,6 +528,7 @@ struct btrfs_ioctl_clone_range_args {
>  				      struct btrfs_ioctl_get_dev_stats)
>  #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
>  				    struct btrfs_ioctl_dev_replace_args)
> +#define BTRFS_IOC_DEDUP_REGISTER	 _IO(BTRFS_IOCTL_MAGIC, 54)

The interface to dedup should be imho similar to what's established with
scrub/balance and so should be the ioctls.

(Same as before, if you're focused on the actual dedup implementation,
add a comment that it's not finalized.)


david
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux