[PATCH 0/6]

[PATCH 0/6]

[Anand Jain]

The below patch set is on top of,

 git://repo.or.cz/btrfs-progs-unstable/devel.git integration-20130710

and is for your kind review coment and integration

Thanks

Anand Jain (6):
  btrfs-progs: close_all_devices() in btrfs-find-root.c does nothing
  btrfs-progs: let user know that devid can be used if path is missing
  btrfs-progs: cmd_start_replace() to use test_dev_for_mkfs()
  btrfs-progs: mkfs.c overwrites fd without appropriate close
  btrfs-progs: avoid write to the disk before sure to create fs
  btrfs-progs: don't have to report ENOMEDIUM error during open

 btrfs-find-root.c |   17 +--------
 cmds-replace.c    |   33 ++--------------
 disk-io.c         |    3 +-
 disk-io.h         |    1 +
 mkfs.c            |  107 +++++++++++++++++++++-------------------------------
 utils.c           |   56 +++++++++++++++++++++++++++-
 utils.h           |    2 +
 7 files changed, 107 insertions(+), 112 deletions(-)

[PATCH 1/6] btrfs-progs: close_all_devices() in btrfs-find-root.c does nothing

[Anand Jain]

close_all_devices() is declared once in disk-io.c and again
in btrfs-find-root.c. The one in latter is completely useless
so delete it.

This patch will fix it.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 btrfs-find-root.c |   17 +----------------
 disk-io.c         |    3 +--
 disk-io.h         |    1 +
 3 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/btrfs-find-root.c b/btrfs-find-root.c
index 253c200..82440f6 100644
--- a/btrfs-find-root.c
+++ b/btrfs-find-root.c
@@ -34,6 +34,7 @@
 #include "volumes.h"
 #include "utils.h"
 #include "crc32c.h"
+#include "disk-io.h"
 
 static u16 csum_size = 0;
 static u64 search_objectid = BTRFS_ROOT_TREE_OBJECTID;
@@ -65,22 +66,6 @@ int csum_block(void *buf, u32 len)
 	return ret;
 }
 
-static int close_all_devices(struct btrfs_fs_info *fs_info)
-{
-	struct list_head *list;
-	struct list_head *next;
-	struct btrfs_device *device;
-
-	return 0;
-
-	list = &fs_info->fs_devices->devices;
-	list_for_each(next, list) {
-		device = list_entry(next, struct btrfs_device, dev_list);
-		close(device->fd);
-	}
-	return 0;
-}
-
 static struct btrfs_root *open_ctree_broken(int fd, const char *device)
 {
 	u32 sectorsize;
diff --git a/disk-io.c b/disk-io.c
index 5b84b02..d23c4d9 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -35,7 +35,6 @@
 #include "utils.h"
 #include "print-tree.h"
 
-static int close_all_devices(struct btrfs_fs_info *fs_info);
 
 static int check_tree_block(struct btrfs_root *root, struct extent_buffer *buf)
 {
@@ -1165,7 +1164,7 @@ int write_ctree_super(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
-static int close_all_devices(struct btrfs_fs_info *fs_info)
+int close_all_devices(struct btrfs_fs_info *fs_info)
 {
 	struct list_head *list;
 	struct btrfs_device *device;
diff --git a/disk-io.h b/disk-io.h
index 0158d17..c25bc07 100644
--- a/disk-io.h
+++ b/disk-io.h
@@ -89,6 +89,7 @@ int csum_tree_block_size(struct extent_buffer *buf, u16 csum_sectorsize,
 int csum_tree_block(struct btrfs_root *root, struct extent_buffer *buf,
 		    int verify);
 int btrfs_read_buffer(struct extent_buffer *buf, u64 parent_transid);
+int close_all_devices(struct btrfs_fs_info *fs_info);
 #endif
 
 /* raid6.c */
-- 
1.7.1

[PATCH 2/6] btrfs-progs: let user know that devid can be used if path is missing

[Anand Jain]

When the device disappear the path goes missing,
and that will be the one of the reason that user
will replace the device.

The devid of the missing btrfs device can be
obtained using the new cli option
 btrfs fi show --kernel

And which can be used in the replace command.

---
btrfs replace start /dev/sdc /dev/sde /btrfs
Error: Unable to open device '/dev/sdc'
	Try using the devid instead of the path
---

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

diff --git a/cmds-replace.c b/cmds-replace.c
index 6397bb5..08a369a 100644
--- a/cmds-replace.c
+++ b/cmds-replace.c
@@ -244,6 +244,7 @@ static int cmd_start_replace(int argc, char **argv)
 		if (fdsrcdev < 0) {
 			fprintf(stderr, "Error: Unable to open device '%s'\n",
 				srcdev);
+			fprintf(stderr, "\tTry using the devid instead of the path\n");
 			goto leave_with_error;
 		}
 		ret = fstat(fdsrcdev, &st);
-- 
1.7.1

[PATCH 3/6] btrfs-progs: cmd_start_replace() to use test_dev_for_mkfs()

[Anand Jain]

test_dev_for_mkfs() is a common place where
we check if a device is fit for the btrfs use.
cmd_start_replace() should make use of test_dev_for_mkfs(),
and here the test_dev_for_mkfs() is further enhanced
to fit the cmd_start_replace() needs.

Thanks

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-replace.c |   32 ++++----------------------------
 utils.c        |   10 ++++++++++
 2 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/cmds-replace.c b/cmds-replace.c
index 08a369a..bda8269 100644
--- a/cmds-replace.c
+++ b/cmds-replace.c
@@ -137,12 +137,11 @@ static int cmd_start_replace(int argc, char **argv)
 	char *dstdev;
 	int avoid_reading_from_srcdev = 0;
 	int force_using_targetdev = 0;
-	u64 total_devs = 1;
-	struct btrfs_fs_devices *fs_devices_mnt = NULL;
 	struct stat st;
 	u64 dstdev_block_count;
 	int do_not_background = 0;
 	int mixed = 0;
+	char estr[100]; /* check test_dev_for_mkfs() for error string size*/
 
 	while ((c = getopt(argc, argv, "Brf")) != -1) {
 		switch (c) {
@@ -264,15 +263,9 @@ static int cmd_start_replace(int argc, char **argv)
 		start_args.start.srcdevid = 0;
 	}
 
-	ret = check_mounted(dstdev);
-	if (ret < 0) {
-		fprintf(stderr, "Error checking %s mount status\n", dstdev);
-		goto leave_with_error;
-	}
-	if (ret == 1) {
-		fprintf(stderr,
-			"Error, target device %s is in use and currently mounted!\n",
-			dstdev);
+	ret = test_dev_for_mkfs(dstdev, force_using_targetdev, estr);
+	if (ret) {
+		fprintf(stderr, "%s", estr);
 		goto leave_with_error;
 	}
 	fddstdev = open(dstdev, O_RDWR);
@@ -280,23 +273,6 @@ static int cmd_start_replace(int argc, char **argv)
 		fprintf(stderr, "Unable to open %s\n", dstdev);
 		goto leave_with_error;
 	}
-	ret = btrfs_scan_one_device(fddstdev, dstdev, &fs_devices_mnt,
-				    &total_devs, BTRFS_SUPER_INFO_OFFSET);
-	if (ret >= 0 && !force_using_targetdev) {
-		fprintf(stderr,
-			"Error, target device %s contains filesystem, use '-f' to force overwriting.\n",
-			dstdev);
-		goto leave_with_error;
-	}
-	ret = fstat(fddstdev, &st);
-	if (ret) {
-		fprintf(stderr, "Error: Unable to stat '%s'\n", dstdev);
-		goto leave_with_error;
-	}
-	if (!S_ISBLK(st.st_mode)) {
-		fprintf(stderr, "Error: '%s' is not a block device\n", dstdev);
-		goto leave_with_error;
-	}
 	strncpy((char *)start_args.start.tgtdev_name, dstdev,
 		BTRFS_DEVICE_PATH_NAME_MAX);
 	if (btrfs_prepare_device(fddstdev, dstdev, 1, &dstdev_block_count, 0,
diff --git a/utils.c b/utils.c
index e8a42bf..f42466f 100644
--- a/utils.c
+++ b/utils.c
@@ -1805,6 +1805,7 @@ int test_dev_for_mkfs(char *file, int force_overwrite, char *estr)
 {
 	int ret, fd;
 	size_t sz = 100;
+	struct stat st;
 
 	ret = is_swap_device(file);
 	if (ret < 0) {
@@ -1839,6 +1840,15 @@ int test_dev_for_mkfs(char *file, int force_overwrite, char *estr)
 			strerror(errno));
 		return 1;
 	}
+	if (fstat(fd, &st)) {
+		snprintf(estr, sz, "unable to stat %s: %s\n", file,
+			strerror(errno));
+		return 1;
+	}
+	if (!S_ISBLK(st.st_mode)) {
+		fprintf(stderr, "'%s' is not a block device\n", file);
+		return 1;
+	}
 	close(fd);
 	return 0;
 }
-- 
1.7.1

[PATCH 4/6] btrfs-progs: mkfs.c overwrites fd without appropriate close

[Anand Jain]

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 mkfs.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/mkfs.c b/mkfs.c
index 60f906c..66f558a 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -1570,6 +1570,8 @@ int main(int ac, char **av)
 		 * occur by the following processing.
 		 * (btrfs_register_one_device() fails if O_EXCL is on)
 		 */
+		if (fd > 0)
+			close(fd);
 		fd = open(file, O_RDWR);
 		if (fd < 0) {
 			fprintf(stderr, "unable to open %s: %s\n", file,
@@ -1581,7 +1583,6 @@ int main(int ac, char **av)
 		if (ret) {
 			fprintf(stderr, "skipping duplicate device %s in FS\n",
 				file);
-			close(fd);
 			continue;
 		}
 		ret = btrfs_prepare_device(fd, file, zero_end, &dev_block_count,
-- 
1.7.1

[PATCH 5/6] btrfs-progs: avoid write to the disk before sure to create fs

[Anand Jain]

This patch provides fix for the following bug,

When mkfs.btrfs fails the disks shouldn't be written.
------------
btrfs fi show /dev/sdb
Label: none  uuid: 60fb76f4-3b4d-4632-a7da-6a44dea5573d
        Total devices 1 FS bytes used 24.00KiB
        devid    1 size 2.00GiB used 20.00MiB path /dev/sdb

mkfs.btrfs -dsingle -mraid1 /dev/sdb -f
::
unable to create FS with metadata profile 16 (have 1 devices)

btrfs fi show /dev/sdb
Label: none  uuid: 2da2179d-ecb1-4a4e-a44d-e7613a08c18d
        Total devices 1 FS bytes used 24.00KiB
        devid    1 size 2.00GiB used 20.00MiB path /dev/sdb
-------------

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 mkfs.c  |  104 +++++++++++++++++++++++++--------------------------------------
 utils.c |   41 +++++++++++++++++++++++++
 utils.h |    2 +
 3 files changed, 84 insertions(+), 63 deletions(-)

diff --git a/mkfs.c b/mkfs.c
index 66f558a..a73a4e2 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -195,83 +195,28 @@ static int create_raid_groups(struct btrfs_trans_handle *trans,
 			      int metadata_profile_opt, int mixed, int ssd)
 {
 	u64 num_devices = btrfs_super_num_devices(root->fs_info->super_copy);
-	u64 allowed = 0;
-	u64 devices_for_raid = num_devices;
 	int ret;
 
-	/*
-	 * Set default profiles according to number of added devices.
-	 * For mixed groups defaults are single/single.
-	 */
-	if (!metadata_profile_opt && !mixed) {
-		if (num_devices == 1 && ssd)
-			printf("Detected a SSD, turning off metadata "
-			       "duplication.  Mkfs with -m dup if you want to "
-			       "force metadata duplication.\n");
-		metadata_profile = (num_devices > 1) ?
-			BTRFS_BLOCK_GROUP_RAID1 : (ssd) ? 0: BTRFS_BLOCK_GROUP_DUP;
-	}
-	if (!data_profile_opt && !mixed) {
-		data_profile = (num_devices > 1) ?
-			BTRFS_BLOCK_GROUP_RAID0 : 0; /* raid0 or single */
-	}
-
-	if (devices_for_raid > 4)
-		devices_for_raid = 4;
-
-	switch (devices_for_raid) {
-	default:
-	case 4:
-		allowed |= BTRFS_BLOCK_GROUP_RAID10;
-	case 3:
-		allowed |= BTRFS_BLOCK_GROUP_RAID6;
-	case 2:
-		allowed |= BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1 |
-			BTRFS_BLOCK_GROUP_RAID5;
-		break;
-	case 1:
-		allowed |= BTRFS_BLOCK_GROUP_DUP;
-	}
-
-	if (metadata_profile & ~allowed) {
-		fprintf(stderr,	"unable to create FS with metadata "
-			"profile %llu (have %llu devices)\n", metadata_profile,
-			num_devices);
-		exit(1);
-	}
-	if (data_profile & ~allowed) {
-		fprintf(stderr, "unable to create FS with data "
-			"profile %llu (have %llu devices)\n", data_profile,
-			num_devices);
-		exit(1);
-	}
-
-	/* allow dup'ed data chunks only in mixed mode */
-	if (!mixed && (data_profile & BTRFS_BLOCK_GROUP_DUP)) {
-		fprintf(stderr, "dup for data is allowed only in mixed mode\n");
-		exit(1);
-	}
-
-	if (allowed & metadata_profile) {
+	if (metadata_profile) {
 		u64 meta_flags = BTRFS_BLOCK_GROUP_METADATA;
 
 		ret = create_one_raid_group(trans, root,
 					    BTRFS_BLOCK_GROUP_SYSTEM |
-					    (allowed & metadata_profile));
+					    metadata_profile);
 		BUG_ON(ret);
 
 		if (mixed)
 			meta_flags |= BTRFS_BLOCK_GROUP_DATA;
 
 		ret = create_one_raid_group(trans, root, meta_flags |
-					    (allowed & metadata_profile));
+					    metadata_profile);
 		BUG_ON(ret);
 
 	}
-	if (!mixed && num_devices > 1 && (allowed & data_profile)) {
+	if (!mixed && num_devices > 1 && data_profile) {
 		ret = create_one_raid_group(trans, root,
 					    BTRFS_BLOCK_GROUP_DATA |
-					    (allowed & data_profile));
+					    data_profile);
 		BUG_ON(ret);
 	}
 	recow_roots(trans, root);
@@ -1464,14 +1409,48 @@ int main(int ac, char **av)
 			}
 	}
 
-	/* if we are here that means all devs are good to btrfsify */
 	optind = saved_optind;
 	dev_cnt = ac - optind;
 
+	file = av[optind++];
+	ssd = is_ssd(file);
+
+	/*
+	* Set default profiles according to number of added devices.
+	* For mixed groups defaults are single/single.
+	*/
+	if (!mixed) {
+		if (!metadata_profile_opt) {
+			if (dev_cnt == 1 && ssd)
+				printf("Detected a SSD, turning off metadata "
+				"duplication.  Mkfs with -m dup if you want to "
+				"force metadata duplication.\n");
+
+			metadata_profile = (dev_cnt > 1) ?
+					BTRFS_BLOCK_GROUP_RAID1 : (ssd) ?
+					0: BTRFS_BLOCK_GROUP_DUP;
+		}
+		if (!data_profile_opt) {
+			data_profile = (dev_cnt > 1) ?
+				BTRFS_BLOCK_GROUP_RAID0 : 0; /* raid0 or single */
+		}
+	} else {
+		/* this is not needed but just for completeness */
+		metadata_profile = 0;
+		data_profile = 0;
+	}
+
+	ret = test_num_disk_vs_raid(metadata_profile, data_profile,
+			dev_cnt, mixed, estr);
+	if (ret) {
+		fprintf(stderr, "Error: %s\n", estr);
+		exit(1);
+	}
+
+	/* if we are here that means all devs are good to btrfsify */
 	printf("\nWARNING! - %s IS EXPERIMENTAL\n", BTRFS_BUILD_VERSION);
 	printf("WARNING! - see http://btrfs.wiki.kernel.org before using\n\n");
 
-	file = av[optind++];
 	dev_cnt--;
 
 	if (!source_dir_set) {
@@ -1514,7 +1493,6 @@ int main(int ac, char **av)
 		dev_block_count = block_count;
 	}
 
-	ssd = is_ssd(file);
 
 	if (mixed) {
 		if (metadata_profile != data_profile) {
diff --git a/utils.c b/utils.c
index f42466f..4aeea9c 100644
--- a/utils.c
+++ b/utils.c
@@ -1796,6 +1796,47 @@ out:
 	return ret;
 }
 
+int test_num_disk_vs_raid(u64 metadata_profile, u64 data_profile,
+	u64 dev_cnt, int mixed, char *estr)
+{
+	size_t sz = 100;
+	u64 allowed = 0;
+
+	switch (dev_cnt) {
+	default:
+	case 4:
+		allowed |= BTRFS_BLOCK_GROUP_RAID10;
+	case 3:
+		allowed |= BTRFS_BLOCK_GROUP_RAID6;
+	case 2:
+		allowed |= BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1 |
+			BTRFS_BLOCK_GROUP_RAID5;
+		break;
+	case 1:
+		allowed |= BTRFS_BLOCK_GROUP_DUP;
+	}
+
+	if (metadata_profile & ~allowed) {
+		snprintf(estr, sz, "unable to create FS with metadata "
+			"profile %llu (have %llu devices)\n",
+			metadata_profile, dev_cnt);
+		return 1;
+	}
+	if (data_profile & ~allowed) {
+		snprintf(estr, sz, "unable to create FS with data "
+			"profile %llu (have %llu devices)\n",
+			metadata_profile, dev_cnt);
+		return 1;
+	}
+
+	if (!mixed && (data_profile & BTRFS_BLOCK_GROUP_DUP)) {
+		snprintf(estr, sz,
+			"dup for data is allowed only in mixed mode");
+		return 1;
+	}
+	return 0;
+}
+
 /* Check if disk is suitable for btrfs
  * returns:
  *  1: something is wrong, estr provides the error
diff --git a/utils.h b/utils.h
index 708b580..b6da40d 100644
--- a/utils.h
+++ b/utils.h
@@ -78,4 +78,6 @@ u64 btrfs_device_size(int fd, struct stat *st);
 int test_dev_for_mkfs(char *file, int force_overwrite, char *estr);
 int scan_for_btrfs(int where, int update_kernel);
 int get_label_mounted(const char *mount_path, char *labelp);
+int test_num_disk_vs_raid(u64 metadata_profile, u64 data_profile,
+	u64 dev_cnt, int mixed, char *estr);
 #endif
-- 
1.7.1

[PATCH 6/6] btrfs-progs: don't have to report ENOMEDIUM error during open

[Anand Jain]

when we scan /proc/partitions the cdrom is scanned
as well, and we don't have to report ENOMEDIUM errors
against it.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 utils.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/utils.c b/utils.c
index 4aeea9c..638cef0 100644
--- a/utils.c
+++ b/utils.c
@@ -1443,8 +1443,9 @@ scan_again:
 
 		fd = open(fullpath, O_RDONLY);
 		if (fd < 0) {
-			fprintf(stderr, "failed to open %s: %s\n",
-				fullpath, strerror(errno));
+			if (errno != ENOMEDIUM)
+				fprintf(stderr, "failed to open %s: %s\n",
+					fullpath, strerror(errno));
 			continue;
 		}
 		ret = btrfs_scan_one_device(fd, fullpath, &tmp_devices,
-- 
1.7.1

[PATCH 0/3 resend]

[Anand Jain]

Anand Jain (3):
  btrfs-progs: let user know that devid can be used if path is missing
  btrfs-progs: cmd_start_replace() to use test_dev_for_mkfs()
  btrfs-progs: avoid write to the disk before sure to create fs

 cmds-replace.c |   33 +++---------------
 mkfs.c         |  104 ++++++++++++++++++++++----------------------------------
 utils.c        |   53 ++++++++++++++++++++++++++++-
 utils.h        |    4 ++
 4 files changed, 102 insertions(+), 92 deletions(-)

[PATCH 1/3] btrfs-progs: let user know that devid can be used if path is missing

[Anand Jain]

When the device disappear the path goes missing,
and that will be the one of the reason that user
will replace the device.

The devid of the missing btrfs device can be
obtained using the new cli option
 btrfs fi show --kernel (coming soon)

And which can be used in the replace command.
---
 cmds-replace.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/cmds-replace.c b/cmds-replace.c
index e409e11..25e8c7f 100644
--- a/cmds-replace.c
+++ b/cmds-replace.c
@@ -245,6 +245,7 @@ static int cmd_start_replace(int argc, char **argv)
 		if (fdsrcdev < 0) {
 			fprintf(stderr, "Error: Unable to open device '%s'\n",
 				srcdev);
+			fprintf(stderr, "\tTry using the devid instead of the path\n");
 			goto leave_with_error;
 		}
 		ret = fstat(fdsrcdev, &st);
-- 
1.7.1

[PATCH 2/3] btrfs-progs: cmd_start_replace() to use test_dev_for_mkfs()

[Anand Jain]

test_dev_for_mkfs() is a common place where
we check if a device is fit for the btrfs use.
cmd_start_replace() should make use of test_dev_for_mkfs(),
and here the test_dev_for_mkfs() is further enhanced
to fit the cmd_start_replace() needs.

Thanks

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-replace.c |   32 ++++----------------------------
 utils.c        |   12 +++++++++++-
 utils.h        |    2 ++
 3 files changed, 17 insertions(+), 29 deletions(-)

diff --git a/cmds-replace.c b/cmds-replace.c
index 25e8c7f..eecea4b 100644
--- a/cmds-replace.c
+++ b/cmds-replace.c
@@ -137,13 +137,12 @@ static int cmd_start_replace(int argc, char **argv)
 	char *dstdev;
 	int avoid_reading_from_srcdev = 0;
 	int force_using_targetdev = 0;
-	u64 total_devs = 1;
-	struct btrfs_fs_devices *fs_devices_mnt = NULL;
 	struct stat st;
 	u64 dstdev_block_count;
 	int do_not_background = 0;
 	int mixed = 0;
 	DIR *dirstream = NULL;
+	char estr[BTRFS_ERR_STR_LEN];
 
 	while ((c = getopt(argc, argv, "Brf")) != -1) {
 		switch (c) {
@@ -265,15 +264,9 @@ static int cmd_start_replace(int argc, char **argv)
 		start_args.start.srcdevid = 0;
 	}
 
-	ret = check_mounted(dstdev);
-	if (ret < 0) {
-		fprintf(stderr, "Error checking %s mount status\n", dstdev);
-		goto leave_with_error;
-	}
-	if (ret == 1) {
-		fprintf(stderr,
-			"Error, target device %s is in use and currently mounted!\n",
-			dstdev);
+	ret = test_dev_for_mkfs(dstdev, force_using_targetdev, estr);
+	if (ret) {
+		fprintf(stderr, "%s", estr);
 		goto leave_with_error;
 	}
 	fddstdev = open(dstdev, O_RDWR);
@@ -281,23 +274,6 @@ static int cmd_start_replace(int argc, char **argv)
 		fprintf(stderr, "Unable to open %s\n", dstdev);
 		goto leave_with_error;
 	}
-	ret = btrfs_scan_one_device(fddstdev, dstdev, &fs_devices_mnt,
-				    &total_devs, BTRFS_SUPER_INFO_OFFSET);
-	if (ret >= 0 && !force_using_targetdev) {
-		fprintf(stderr,
-			"Error, target device %s contains filesystem, use '-f' to force overwriting.\n",
-			dstdev);
-		goto leave_with_error;
-	}
-	ret = fstat(fddstdev, &st);
-	if (ret) {
-		fprintf(stderr, "Error: Unable to stat '%s'\n", dstdev);
-		goto leave_with_error;
-	}
-	if (!S_ISBLK(st.st_mode)) {
-		fprintf(stderr, "Error: '%s' is not a block device\n", dstdev);
-		goto leave_with_error;
-	}
 	strncpy((char *)start_args.start.tgtdev_name, dstdev,
 		BTRFS_DEVICE_PATH_NAME_MAX);
 	if (btrfs_prepare_device(fddstdev, dstdev, 1, &dstdev_block_count, 0,
diff --git a/utils.c b/utils.c
index 15b991f..e35529e 100644
--- a/utils.c
+++ b/utils.c
@@ -1806,7 +1806,8 @@ out:
 int test_dev_for_mkfs(char *file, int force_overwrite, char *estr)
 {
 	int ret, fd;
-	size_t sz = 100;
+	size_t sz = BTRFS_ERR_STR_LEN;
+	struct stat st;
 
 	ret = is_swap_device(file);
 	if (ret < 0) {
@@ -1841,6 +1842,15 @@ int test_dev_for_mkfs(char *file, int force_overwrite, char *estr)
 			strerror(errno));
 		return 1;
 	}
+	if (fstat(fd, &st)) {
+		snprintf(estr, sz, "unable to stat %s: %s\n", file,
+			strerror(errno));
+		return 1;
+	}
+	if (!S_ISBLK(st.st_mode)) {
+		fprintf(stderr, "'%s' is not a block device\n", file);
+		return 1;
+	}
 	close(fd);
 	return 0;
 }
diff --git a/utils.h b/utils.h
index 5b95f3b..1e5353b 100644
--- a/utils.h
+++ b/utils.h
@@ -28,6 +28,8 @@
 #define BTRFS_SCAN_PROC	1
 #define BTRFS_SCAN_DEV		2
 
+#define BTRFS_ERR_STR_LEN	100
+
 int make_btrfs(int fd, const char *device, const char *label,
 	       u64 blocks[6], u64 num_bytes, u32 nodesize,
 	       u32 leafsize, u32 sectorsize, u32 stripesize);
-- 
1.7.1

[PATCH 3/3] btrfs-progs: avoid write to the disk before sure to create fs

[Anand Jain]

This patch provides fix for the following bug,

When mkfs.btrfs fails the disks shouldn't be written.
------------
btrfs fi show /dev/sdb
Label: none  uuid: 60fb76f4-3b4d-4632-a7da-6a44dea5573d
        Total devices 1 FS bytes used 24.00KiB
        devid    1 size 2.00GiB used 20.00MiB path /dev/sdb

mkfs.btrfs -dsingle -mraid1 /dev/sdb -f
::
unable to create FS with metadata profile 16 (have 1 devices)

btrfs fi show /dev/sdb
Label: none  uuid: 2da2179d-ecb1-4a4e-a44d-e7613a08c18d
        Total devices 1 FS bytes used 24.00KiB
        devid    1 size 2.00GiB used 20.00MiB path /dev/sdb
-------------

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 mkfs.c  |  104 +++++++++++++++++++++++++--------------------------------------
 utils.c |   41 +++++++++++++++++++++++++
 utils.h |    2 +
 3 files changed, 84 insertions(+), 63 deletions(-)

diff --git a/mkfs.c b/mkfs.c
index 6325c1f..d01d08b 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -195,83 +195,28 @@ static int create_raid_groups(struct btrfs_trans_handle *trans,
 			      int metadata_profile_opt, int mixed, int ssd)
 {
 	u64 num_devices = btrfs_super_num_devices(root->fs_info->super_copy);
-	u64 allowed = 0;
-	u64 devices_for_raid = num_devices;
 	int ret;
 
-	/*
-	 * Set default profiles according to number of added devices.
-	 * For mixed groups defaults are single/single.
-	 */
-	if (!metadata_profile_opt && !mixed) {
-		if (num_devices == 1 && ssd)
-			printf("Detected a SSD, turning off metadata "
-			       "duplication.  Mkfs with -m dup if you want to "
-			       "force metadata duplication.\n");
-		metadata_profile = (num_devices > 1) ?
-			BTRFS_BLOCK_GROUP_RAID1 : (ssd) ? 0: BTRFS_BLOCK_GROUP_DUP;
-	}
-	if (!data_profile_opt && !mixed) {
-		data_profile = (num_devices > 1) ?
-			BTRFS_BLOCK_GROUP_RAID0 : 0; /* raid0 or single */
-	}
-
-	if (devices_for_raid > 4)
-		devices_for_raid = 4;
-
-	switch (devices_for_raid) {
-	default:
-	case 4:
-		allowed |= BTRFS_BLOCK_GROUP_RAID10;
-	case 3:
-		allowed |= BTRFS_BLOCK_GROUP_RAID6;
-	case 2:
-		allowed |= BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1 |
-			BTRFS_BLOCK_GROUP_RAID5;
-		break;
-	case 1:
-		allowed |= BTRFS_BLOCK_GROUP_DUP;
-	}
-
-	if (metadata_profile & ~allowed) {
-		fprintf(stderr,	"unable to create FS with metadata "
-			"profile %llu (have %llu devices)\n", metadata_profile,
-			num_devices);
-		exit(1);
-	}
-	if (data_profile & ~allowed) {
-		fprintf(stderr, "unable to create FS with data "
-			"profile %llu (have %llu devices)\n", data_profile,
-			num_devices);
-		exit(1);
-	}
-
-	/* allow dup'ed data chunks only in mixed mode */
-	if (!mixed && (data_profile & BTRFS_BLOCK_GROUP_DUP)) {
-		fprintf(stderr, "dup for data is allowed only in mixed mode\n");
-		exit(1);
-	}
-
-	if (allowed & metadata_profile) {
+	if (metadata_profile) {
 		u64 meta_flags = BTRFS_BLOCK_GROUP_METADATA;
 
 		ret = create_one_raid_group(trans, root,
 					    BTRFS_BLOCK_GROUP_SYSTEM |
-					    (allowed & metadata_profile));
+					    metadata_profile);
 		BUG_ON(ret);
 
 		if (mixed)
 			meta_flags |= BTRFS_BLOCK_GROUP_DATA;
 
 		ret = create_one_raid_group(trans, root, meta_flags |
-					    (allowed & metadata_profile));
+					    metadata_profile);
 		BUG_ON(ret);
 
 	}
-	if (!mixed && num_devices > 1 && (allowed & data_profile)) {
+	if (!mixed && num_devices > 1 && data_profile) {
 		ret = create_one_raid_group(trans, root,
 					    BTRFS_BLOCK_GROUP_DATA |
-					    (allowed & data_profile));
+					    data_profile);
 		BUG_ON(ret);
 	}
 	recow_roots(trans, root);
@@ -1464,14 +1409,48 @@ int main(int ac, char **av)
 			}
 	}
 
-	/* if we are here that means all devs are good to btrfsify */
 	optind = saved_optind;
 	dev_cnt = ac - optind;
 
+	file = av[optind++];
+	ssd = is_ssd(file);
+
+	/*
+	* Set default profiles according to number of added devices.
+	* For mixed groups defaults are single/single.
+	*/
+	if (!mixed) {
+		if (!metadata_profile_opt) {
+			if (dev_cnt == 1 && ssd)
+				printf("Detected a SSD, turning off metadata "
+				"duplication.  Mkfs with -m dup if you want to "
+				"force metadata duplication.\n");
+
+			metadata_profile = (dev_cnt > 1) ?
+					BTRFS_BLOCK_GROUP_RAID1 : (ssd) ?
+					0: BTRFS_BLOCK_GROUP_DUP;
+		}
+		if (!data_profile_opt) {
+			data_profile = (dev_cnt > 1) ?
+				BTRFS_BLOCK_GROUP_RAID0 : 0; /* raid0 or single */
+		}
+	} else {
+		/* this is not needed but just for completeness */
+		metadata_profile = 0;
+		data_profile = 0;
+	}
+
+	ret = test_num_disk_vs_raid(metadata_profile, data_profile,
+			dev_cnt, mixed, estr);
+	if (ret) {
+		fprintf(stderr, "Error: %s\n", estr);
+		exit(1);
+	}
+
+	/* if we are here that means all devs are good to btrfsify */
 	printf("\nWARNING! - %s IS EXPERIMENTAL\n", BTRFS_BUILD_VERSION);
 	printf("WARNING! - see http://btrfs.wiki.kernel.org before using\n\n");
 
-	file = av[optind++];
 	dev_cnt--;
 
 	if (!source_dir_set) {
@@ -1514,7 +1493,6 @@ int main(int ac, char **av)
 		dev_block_count = block_count;
 	}
 
-	ssd = is_ssd(file);
 
 	if (mixed) {
 		if (metadata_profile != data_profile) {
diff --git a/utils.c b/utils.c
index e35529e..8a57967 100644
--- a/utils.c
+++ b/utils.c
@@ -1798,6 +1798,47 @@ out:
 	return ret;
 }
 
+int test_num_disk_vs_raid(u64 metadata_profile, u64 data_profile,
+	u64 dev_cnt, int mixed, char *estr)
+{
+	size_t sz = 100;
+	u64 allowed = 0;
+
+	switch (dev_cnt) {
+	default:
+	case 4:
+		allowed |= BTRFS_BLOCK_GROUP_RAID10;
+	case 3:
+		allowed |= BTRFS_BLOCK_GROUP_RAID6;
+	case 2:
+		allowed |= BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1 |
+			BTRFS_BLOCK_GROUP_RAID5;
+		break;
+	case 1:
+		allowed |= BTRFS_BLOCK_GROUP_DUP;
+	}
+
+	if (metadata_profile & ~allowed) {
+		snprintf(estr, sz, "unable to create FS with metadata "
+			"profile %llu (have %llu devices)\n",
+			metadata_profile, dev_cnt);
+		return 1;
+	}
+	if (data_profile & ~allowed) {
+		snprintf(estr, sz, "unable to create FS with data "
+			"profile %llu (have %llu devices)\n",
+			metadata_profile, dev_cnt);
+		return 1;
+	}
+
+	if (!mixed && (data_profile & BTRFS_BLOCK_GROUP_DUP)) {
+		snprintf(estr, sz,
+			"dup for data is allowed only in mixed mode");
+		return 1;
+	}
+	return 0;
+}
+
 /* Check if disk is suitable for btrfs
  * returns:
  *  1: something is wrong, estr provides the error
diff --git a/utils.h b/utils.h
index 1e5353b..6419c3e 100644
--- a/utils.h
+++ b/utils.h
@@ -80,4 +80,6 @@ u64 btrfs_device_size(int fd, struct stat *st);
 int test_dev_for_mkfs(char *file, int force_overwrite, char *estr);
 int scan_for_btrfs(int where, int update_kernel);
 int get_label_mounted(const char *mount_path, char *labelp);
+int test_num_disk_vs_raid(u64 metadata_profile, u64 data_profile,
+	u64 dev_cnt, int mixed, char *estr);
 #endif
-- 
1.7.1

Re: [PATCH 4/6] btrfs-progs: mkfs.c overwrites fd without appropriate close

[Josef Bacik]

On Fri, Jul 26, 2013 at 01:35:28AM +0800, Anand Jain wrote:
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  mkfs.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/mkfs.c b/mkfs.c
> index 60f906c..66f558a 100644
> --- a/mkfs.c
> +++ b/mkfs.c
> @@ -1570,6 +1570,8 @@ int main(int ac, char **av)
>  		 * occur by the following processing.
>  		 * (btrfs_register_one_device() fails if O_EXCL is on)
>  		 */
> +		if (fd > 0)
> +			close(fd);
>  		fd = open(file, O_RDWR);
>  		if (fd < 0) {
>  			fprintf(stderr, "unable to open %s: %s\n", file,
> @@ -1581,7 +1583,6 @@ int main(int ac, char **av)
>  		if (ret) {
>  			fprintf(stderr, "skipping duplicate device %s in FS\n",
>  				file);
> -			close(fd);
>  			continue;
>  		}
>  		ret = btrfs_prepare_device(fd, file, zero_end, &dev_block_count,

This breaks mkfs with multiple disks.  Please for the love of all that is holy
just do a xfstests run with your patches to make sure they don't break anything
so when I go to try to test something completely different I don't have to waste
time bisecting down to figure out wtf you broke today?  David can you kick this
one out of integration for the time being please?  Thanks,

Josef

Re: [PATCH 4/6] btrfs-progs: mkfs.c overwrites fd without appropriate close

[Josef Bacik]

On Tue, Aug 13, 2013 at 03:14:08PM -0400, Josef Bacik wrote:
> On Fri, Jul 26, 2013 at 01:35:28AM +0800, Anand Jain wrote:
> > Signed-off-by: Anand Jain <anand.jain@oracle.com>
> > ---
> >  mkfs.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > diff --git a/mkfs.c b/mkfs.c
> > index 60f906c..66f558a 100644
> > --- a/mkfs.c
> > +++ b/mkfs.c
> > @@ -1570,6 +1570,8 @@ int main(int ac, char **av)
> >  		 * occur by the following processing.
> >  		 * (btrfs_register_one_device() fails if O_EXCL is on)
> >  		 */
> > +		if (fd > 0)
> > +			close(fd);
> >  		fd = open(file, O_RDWR);
> >  		if (fd < 0) {
> >  			fprintf(stderr, "unable to open %s: %s\n", file,
> > @@ -1581,7 +1583,6 @@ int main(int ac, char **av)
> >  		if (ret) {
> >  			fprintf(stderr, "skipping duplicate device %s in FS\n",
> >  				file);
> > -			close(fd);
> >  			continue;
> >  		}
> >  		ret = btrfs_prepare_device(fd, file, zero_end, &dev_block_count,
> 
> This breaks mkfs with multiple disks.  Please for the love of all that is holy
> just do a xfstests run with your patches to make sure they don't break anything
> so when I go to try to test something completely different I don't have to waste
> time bisecting down to figure out wtf you broke today?  David can you kick this
> one out of integration for the time being please?  Thanks,
> 

In fact this isn't right at all, we pass this fd into the device, so we aren't
"overwriting" it, we're assigning it to the device and moving on to the next
thing, and then the close_ctree() stuff closes all the devices as normal.  So
just no, this isn't right at all and can be evicted permanently.  Thanks,

Josef

Re: [PATCH 4/6] btrfs-progs: mkfs.c overwrites fd without appropriate close

[Anand Jain]

On 08/14/2013 03:14 AM, Josef Bacik wrote:
> On Fri, Jul 26, 2013 at 01:35:28AM +0800, Anand Jain wrote:
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   mkfs.c |    3 ++-
>>   1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/mkfs.c b/mkfs.c
>> index 60f906c..66f558a 100644
>> --- a/mkfs.c
>> +++ b/mkfs.c
>> @@ -1570,6 +1570,8 @@ int main(int ac, char **av)
>>   		 * occur by the following processing.
>>   		 * (btrfs_register_one_device() fails if O_EXCL is on)
>>   		 */
>> +		if (fd > 0)
>> +			close(fd);
>>   		fd = open(file, O_RDWR);
>>   		if (fd < 0) {
>>   			fprintf(stderr, "unable to open %s: %s\n", file,
>> @@ -1581,7 +1583,6 @@ int main(int ac, char **av)
>>   		if (ret) {
>>   			fprintf(stderr, "skipping duplicate device %s in FS\n",
>>   				file);
>> -			close(fd);
>>   			continue;
>>   		}
>>   		ret = btrfs_prepare_device(fd, file, zero_end, &dev_block_count,
>
> This breaks mkfs with multiple disks.

  I can't believe as I have been playing with multiple disks
  quite a lot recently. let me dig more.

  Anand

Re: [PATCH 4/6] btrfs-progs: mkfs.c overwrites fd without appropriate close

[Anand Jain]

On 08/14/2013 10:04 AM, Anand Jain wrote:
>
>
> On 08/14/2013 03:14 AM, Josef Bacik wrote:
>> On Fri, Jul 26, 2013 at 01:35:28AM +0800, Anand Jain wrote:
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> ---
>>>   mkfs.c |    3 ++-
>>>   1 files changed, 2 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/mkfs.c b/mkfs.c
>>> index 60f906c..66f558a 100644
>>> --- a/mkfs.c
>>> +++ b/mkfs.c
>>> @@ -1570,6 +1570,8 @@ int main(int ac, char **av)
>>>            * occur by the following processing.
>>>            * (btrfs_register_one_device() fails if O_EXCL is on)
>>>            */
>>> +        if (fd > 0)
>>> +            close(fd);
>>>           fd = open(file, O_RDWR);
>>>           if (fd < 0) {
>>>               fprintf(stderr, "unable to open %s: %s\n", file,
>>> @@ -1581,7 +1583,6 @@ int main(int ac, char **av)
>>>           if (ret) {
>>>               fprintf(stderr, "skipping duplicate device %s in FS\n",
>>>                   file);
>>> -            close(fd);
>>>               continue;
>>>           }
>>>           ret = btrfs_prepare_device(fd, file, zero_end,
>>> &dev_block_count,
>>
>> This breaks mkfs with multiple disks.
>
>   I can't believe as I have been playing with multiple disks
>   quite a lot recently. let me dig more.

  Sorry my mistake.

  Indeed further down btrfs_add_to_fsid() stores fd. closing a
  stored fd is not correct theoretically.

  Josef, Would be keen to know which xfstest caught this. ?

  I am sending a patch to back out this, to be applied on the
  current integration branch if it helps users for the time being.

Thanks, Anand

[PATCH] btrfs-progs: Fix: mkfs.c overwrites fd without appropriate close patch

[Anand Jain]

 btrfs_add_to_fsid() saves the fd in the device list.
 close_ctree() will retrive the device list to handle
 the close(). So the device fd shouldn't closed here.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 mkfs.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/mkfs.c b/mkfs.c
index 8183879..73f5425 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -1548,8 +1548,6 @@ int main(int ac, char **av)
 		 * occur by the following processing.
 		 * (btrfs_register_one_device() fails if O_EXCL is on)
 		 */
-		if (fd > 0)
-			close(fd);
 		fd = open(file, O_RDWR);
 		if (fd < 0) {
 			fprintf(stderr, "unable to open %s: %s\n", file,
@@ -1561,6 +1559,7 @@ int main(int ac, char **av)
 		if (ret) {
 			fprintf(stderr, "skipping duplicate device %s in FS\n",
 				file);
+			close(fd);
 			continue;
 		}
 		ret = btrfs_prepare_device(fd, file, zero_end, &dev_block_count,
-- 
1.7.1

Re: [PATCH 4/6] btrfs-progs: mkfs.c overwrites fd without appropriate close

[Josef Bacik]

On Wed, Aug 14, 2013 at 11:17:05AM +0800, Anand Jain wrote:
> 
> 
> On 08/14/2013 10:04 AM, Anand Jain wrote:
> >
> >
> >On 08/14/2013 03:14 AM, Josef Bacik wrote:
> >>On Fri, Jul 26, 2013 at 01:35:28AM +0800, Anand Jain wrote:
> >>>Signed-off-by: Anand Jain <anand.jain@oracle.com>
> >>>---
> >>>  mkfs.c |    3 ++-
> >>>  1 files changed, 2 insertions(+), 1 deletions(-)
> >>>
> >>>diff --git a/mkfs.c b/mkfs.c
> >>>index 60f906c..66f558a 100644
> >>>--- a/mkfs.c
> >>>+++ b/mkfs.c
> >>>@@ -1570,6 +1570,8 @@ int main(int ac, char **av)
> >>>           * occur by the following processing.
> >>>           * (btrfs_register_one_device() fails if O_EXCL is on)
> >>>           */
> >>>+        if (fd > 0)
> >>>+            close(fd);
> >>>          fd = open(file, O_RDWR);
> >>>          if (fd < 0) {
> >>>              fprintf(stderr, "unable to open %s: %s\n", file,
> >>>@@ -1581,7 +1583,6 @@ int main(int ac, char **av)
> >>>          if (ret) {
> >>>              fprintf(stderr, "skipping duplicate device %s in FS\n",
> >>>                  file);
> >>>-            close(fd);
> >>>              continue;
> >>>          }
> >>>          ret = btrfs_prepare_device(fd, file, zero_end,
> >>>&dev_block_count,
> >>
> >>This breaks mkfs with multiple disks.
> >
> >  I can't believe as I have been playing with multiple disks
> >  quite a lot recently. let me dig more.
> 
>  Sorry my mistake.
> 
>  Indeed further down btrfs_add_to_fsid() stores fd. closing a
>  stored fd is not correct theoretically.
> 
>  Josef, Would be keen to know which xfstest caught this. ?
> 

It was btrfs/265, the one that does raid tests and such.

Josef

Re: [PATCH 3/3] btrfs-progs: avoid write to the disk before sure to create fs

[Josef Bacik]

On Wed, Aug 07, 2013 at 08:11:25PM +0800, Anand Jain wrote:
> This patch provides fix for the following bug,
> 
> When mkfs.btrfs fails the disks shouldn't be written.
> ------------
> btrfs fi show /dev/sdb
> Label: none  uuid: 60fb76f4-3b4d-4632-a7da-6a44dea5573d
>         Total devices 1 FS bytes used 24.00KiB
>         devid    1 size 2.00GiB used 20.00MiB path /dev/sdb
> 
> mkfs.btrfs -dsingle -mraid1 /dev/sdb -f
> ::
> unable to create FS with metadata profile 16 (have 1 devices)
> 
> btrfs fi show /dev/sdb
> Label: none  uuid: 2da2179d-ecb1-4a4e-a44d-e7613a08c18d
>         Total devices 1 FS bytes used 24.00KiB
>         devid    1 size 2.00GiB used 20.00MiB path /dev/sdb
> -------------
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

This regresses making a filesystem that is forced to be a mixed block group.
Dave this is in your integration branch, can you please kick it out?  Anand if
I have to waste 30 more minutes bisecting your latest regression I'm going to
just start ignoring all of your patches.

Josef

Re: [PATCH 3/3] btrfs-progs: avoid write to the disk before sure to create fs

[Anand Jain]

On 08/21/2013 03:19 AM, Josef Bacik wrote:
> On Wed, Aug 07, 2013 at 08:11:25PM +0800, Anand Jain wrote:
>> This patch provides fix for the following bug,
>>
>> When mkfs.btrfs fails the disks shouldn't be written.
>> ------------
>> btrfs fi show /dev/sdb
>> Label: none  uuid: 60fb76f4-3b4d-4632-a7da-6a44dea5573d
>>          Total devices 1 FS bytes used 24.00KiB
>>          devid    1 size 2.00GiB used 20.00MiB path /dev/sdb
>>
>> mkfs.btrfs -dsingle -mraid1 /dev/sdb -f
>> ::
>> unable to create FS with metadata profile 16 (have 1 devices)
>>
>> btrfs fi show /dev/sdb
>> Label: none  uuid: 2da2179d-ecb1-4a4e-a44d-e7613a08c18d
>>          Total devices 1 FS bytes used 24.00KiB
>>          devid    1 size 2.00GiB used 20.00MiB path /dev/sdb
>> -------------
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>
> This regresses making a filesystem that is forced to be a mixed block group.

  I guess you are talking about the regression which was addressed
  by the below patch:

     [PATCH] btrfs-progs: mkfs should check for small vol well before

  If this doesn't help, kindly provide the test case.
  I tried with the usual forced Mixed option it just works fine.