[PATCH] btrfs-progs: device add should check existing FS before adding

[PATCH] btrfs-progs: device add should check existing FS before adding

[Anand Jain]

as of now, when 'btrfs device add' adds a device it doesn't
check if the given device contains an existing FS. This
patch will change that to check the same. which when true
add will fail, and ask user to use -f option to overwrite.

further, since now we have test_dev_for_mkfs() function
to check if a disk can be used, so this patch will also
use this function to test the given device before adding.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-device.c |   44 ++++++++++++++++++--------------------------
 1 files changed, 18 insertions(+), 26 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index 282590c..8446502 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -53,12 +53,25 @@ static const char * const cmd_add_dev_usage[] = {
 static int cmd_add_dev(int argc, char **argv)
 {
 	char	*mntpnt;
-	int	i, fdmnt, ret=0, e;
+	int	i = 1, fdmnt, ret = 0, e;
 	DIR	*dirstream = NULL;
+	int	c, force = 0;
+	char	estr[100];
 
 	if (check_argc_min(argc, 3))
 		usage(cmd_add_dev_usage);
 
+	while ((c = getopt(argc, argv, "f")) != -1) {
+		switch (c) {
+		case 'f':
+			force = 1;
+			i++;
+			break;
+		default:
+			usage(cmd_add_dev_usage);
+		}
+	}
+
 	mntpnt = argv[argc - 1];
 
 	fdmnt = open_file_or_dir(mntpnt, &dirstream);
@@ -67,23 +80,15 @@ static int cmd_add_dev(int argc, char **argv)
 		return 12;
 	}
 
-	for (i = 1; i < argc - 1; i++ ){
+	for (; i < argc - 1; i++) {
 		struct btrfs_ioctl_vol_args ioctl_args;
 		int	devfd, res;
 		u64 dev_block_count = 0;
-		struct stat st;
 		int mixed = 0;
 
-		res = check_mounted(argv[i]);
-		if (res < 0) {
-			fprintf(stderr, "error checking %s mount status\n",
-				argv[i]);
-			ret++;
-			continue;
-		}
-		if (res == 1) {
-			fprintf(stderr, "%s is mounted\n", argv[i]);
-			ret++;
+		res = test_dev_for_mkfs(argv[i], force, estr);
+		if (res) {
+			fprintf(stderr, "%s", estr);
 			continue;
 		}
 
@@ -93,19 +98,6 @@ static int cmd_add_dev(int argc, char **argv)
 			ret++;
 			continue;
 		}
-		res = fstat(devfd, &st);
-		if (res) {
-			fprintf(stderr, "ERROR: Unable to stat '%s'\n", argv[i]);
-			close(devfd);
-			ret++;
-			continue;
-		}
-		if (!S_ISBLK(st.st_mode)) {
-			fprintf(stderr, "ERROR: '%s' is not a block device\n", argv[i]);
-			close(devfd);
-			ret++;
-			continue;
-		}
 
 		res = btrfs_prepare_device(devfd, argv[i], 1, &dev_block_count,
 					   0, &mixed, 0);
-- 
1.7.1

Re: [PATCH] btrfs-progs: device add should check existing FS before adding

[David Sterba]

On Fri, Aug 30, 2013 at 07:09:01PM +0800, Anand Jain wrote:
> as of now, when 'btrfs device add' adds a device it doesn't
> check if the given device contains an existing FS. This
> patch will change that to check the same. which when true
> add will fail, and ask user to use -f option to overwrite.
> 
> further, since now we have test_dev_for_mkfs() function
> to check if a disk can be used, so this patch will also
> use this function to test the given device before adding.

Patch is ok, but the argument handling does not use the common pattern:

> --- a/cmds-device.c
> +++ b/cmds-device.c
> @@ -53,12 +53,25 @@ static const char * const cmd_add_dev_usage[] = {
>  static int cmd_add_dev(int argc, char **argv)
>  {
>  	char	*mntpnt;
> -	int	i, fdmnt, ret=0, e;
> +	int	i = 1, fdmnt, ret = 0, e;
>  	DIR	*dirstream = NULL;
> +	int	c, force = 0;
> +	char	estr[100];
>  
>  	if (check_argc_min(argc, 3))
>  		usage(cmd_add_dev_usage);

check_argc_min belongs after argument processing

> +	while ((c = getopt(argc, argv, "f")) != -1) {
> +		switch (c) {
> +		case 'f':
> +			force = 1;
> +			i++;
> +			break;
> +		default:
> +			usage(cmd_add_dev_usage);
> +		}
> +	}

	argc -= optind;

	if (check_argc_min(...))
		usage();

> +
>  	mntpnt = argv[argc - 1];

And here it gets more complicated as you'd have to add optind everywhere
argc is used.

I was working on a patch to add the --nodiscard parameter to 'device
add' so the work is done, but not yet posted:

http://repo.or.cz/w/btrfs-progs-unstable/devel.git/shortlog/refs/heads/dev/dev-add-notrim

Please base your patch on top of that or after when they hit the
mailinglist/integration.

david

Re: [PATCH] btrfs-progs: device add should check existing FS before adding

[Anand Jain]

> http://repo.or.cz/w/btrfs-progs-unstable/devel.git/shortlog/refs/heads/dev/dev-add-notrim
>
> Please base your patch on top of that or after when they hit the
> mailinglist/integration.

  it makes sense to write patch on top of this. I would wait.

Anand

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