thr3ads.net - Btrfs devel - [PATCH] btrfs-progs: mkfs should check for small vol well before [Aug 2013]

If this information is useful, please help other people find it:
Share via:

Anand Jain

2013-Aug-16 12:52 UTC

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

This fix the regression introduced by 830427d
that it no more creates the FS if disk is small
and if no mixed option is provided.
This patch will bring it to the original design
which will force mixed profile when disk is small
and go ahead to create the FS.

Which also means that before we open the device
for the write we should also check if disk is small.

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

diff --git a/mkfs.c b/mkfs.c
index 73f5425..e58e5c0 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -1415,6 +1415,17 @@ int main(int ac, char **av)
 	file = av[optind++];
 	ssd = is_ssd(file);
 
+	if (is_vol_small(file)) {
+		printf("SMALL VOLUME: forcing mixed metadata/data groups\n");
+		mixed = 1;
+		if (metadata_profile != data_profile) {
+			if (metadata_profile_opt || data_profile_opt) {
+				fprintf(stderr, "With mixed block groups data and metadata "
+					"profiles must be the same\n");
+				exit(1);
+			}
+		}
+	}
 	/*
 	* Set default profiles according to number of added devices.
 	* For mixed groups defaults are single/single.
@@ -1435,7 +1446,6 @@ int main(int ac, char **av)
 				BTRFS_BLOCK_GROUP_RAID0 : 0; /* raid0 or single */
 		}
 	} else {
-		/* this is not needed but just for completeness */
 		metadata_profile = 0;
 		data_profile = 0;
 	}
@@ -1494,14 +1504,6 @@ int main(int ac, char **av)
 	}
 
 
-	if (mixed) {
-		if (metadata_profile != data_profile) {
-			fprintf(stderr, "With mixed block groups data and metadata "
-				"profiles must be the same\n");
-			exit(1);
-		}
-	}
-
 	blocks[0] = BTRFS_SUPER_INFO_OFFSET;
 	for (i = 1; i < 7; i++) {
 		blocks[i] = BTRFS_SUPER_INFO_OFFSET + 1024 * 1024 +
diff --git a/utils.c b/utils.c
index c26193e..422530d 100644
--- a/utils.c
+++ b/utils.c
@@ -1963,3 +1963,32 @@ int scan_for_btrfs(int where, int update_kernel)
 	}
 	return ret;
 }
+
+int is_vol_small(char *file)
+{
+	int fd = -1;
+	int e;
+	struct stat st;
+	u64 size;
+
+	fd = open(file, O_RDONLY);
+	if (fd < 0)
+		return -errno;
+	if (fstat(fd, &st) < 0) {
+		e = -errno;
+		close(fd);
+		return e;
+	}
+	size = btrfs_device_size(fd, &st);
+	if (size == 0) {
+		close(fd);
+		return -1;
+	}
+	if (size < 1024 * 1024 * 1024) {
+		close(fd);
+		return 1;
+	} else {
+		close(fd);
+		return 0;
+	}
+}
diff --git a/utils.h b/utils.h
index 4b449b8..eb6fba3 100644
--- a/utils.h
+++ b/utils.h
@@ -82,4 +82,5 @@ 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);
 int get_btrfs_mount(const char *dev, char *mp, size_t mp_size);
+int is_vol_small(char *file);
 #endif
-- 
1.7.1

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

Anand Jain

2013-Aug-30 08:42 UTC

head link

[PATCH] btrfs-progs: use btrfs error code for kernel errors

Now with the below kernel patch, the excl operations like dev
add/replace/resize and balance returns the btrfs error
code defined in btrfs.h, this patch will help btrfs-progs
(and thus user) to know the error string on the terminal
(instead of /var/log/messages as previously kernel did).

This patch depends on the btrfs kernel patch:
  btrfs: return btrfs error code for dev excl ops err

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-balance.c    |   15 +++++++++++++++
 cmds-device.c     |    7 ++++++-
 cmds-filesystem.c |    6 +++++-
 cmds-replace.c    |    7 ++++++-
 4 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/cmds-balance.c b/cmds-balance.c
index c78b726..30019be 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -316,6 +316,12 @@ static int do_balance(const char *path, struct
btrfs_ioctl_balance_args *args,
 			if (ret == 0)
 				goto out;
 			e = errno;
+			if (ret > 0) {
+				fprintf(stderr,
+					"ERROR: Balance failed due to - %s\n",
+					btrfs_err_str(ret));
+				goto out;
+			}
 		}
 
 		if (e == ECANCELED) {
@@ -332,6 +338,11 @@ static int do_balance(const char *path, struct
btrfs_ioctl_balance_args *args,
 					"syslog - try dmesg | tail\n");
 			ret = 19;
 		}
+	} else if (ret > 0) {
+		fprintf(stderr,
+			"ERROR: Balance start failed - %s\n",
+			btrfs_err_str(ret));
+		ret = 1;
 	} else {
 		printf("Done, had to relocate %llu out of %llu chunks\n",
 		       (unsigned long long)args->stat.completed,
@@ -603,6 +614,10 @@ static int cmd_balance_resume(int argc, char **argv)
 "There may be more info in syslog - try dmesg | tail\n", path,
strerror(e));
 			return 19;
 		}
+	} else if (ret > 0) {
+		fprintf(stderr,
+			"ERROR: Balance resume failed - %s\n",
+			btrfs_err_str(ret));
 	} else {
 		printf("Done, had to relocate %llu out of %llu chunks\n",
 		       (unsigned long long)args.stat.completed,
diff --git a/cmds-device.c b/cmds-device.c
index 833ad30..282590c 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -120,10 +120,15 @@ static int cmd_add_dev(int argc, char **argv)
 		strncpy_null(ioctl_args.name, argv[i]);
 		res = ioctl(fdmnt, BTRFS_IOC_ADD_DEV, &ioctl_args);
 		e = errno;
-		if(res<0){
+		if (res < 0) {
 			fprintf(stderr, "ERROR: error adding the device ''%s''
- %s\n",
 				argv[i], strerror(e));
 			ret++;
+		} else if (res > 0) {
+			fprintf(stderr,
+				"ERROR: adding the device ''%s'' - %s\n",
+				argv[i], btrfs_err_str(res));
+			ret = 1;
 		}
 
 	}
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 412f793..2f6a0ab 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -577,10 +577,14 @@ static int cmd_resize(int argc, char **argv)
 	res = ioctl(fd, BTRFS_IOC_RESIZE, &args);
 	e = errno;
 	close_file_or_dir(fd, dirstream);
-	if( res < 0 ){
+	if (res < 0) {
 		fprintf(stderr, "ERROR: unable to resize ''%s'' -
%s\n",
 			path, strerror(e));
 		return 30;
+	} else if (res > 0) {
+		fprintf(stderr, "ERROR: resize failed - %s\n",
+			btrfs_err_str(res));
+		return res;
 	}
 	return 0;
 }
diff --git a/cmds-replace.c b/cmds-replace.c
index 8ed92c4..e3ff695 100644
--- a/cmds-replace.c
+++ b/cmds-replace.c
@@ -297,12 +297,17 @@ static int cmd_start_replace(int argc, char **argv)
 	start_args.cmd = BTRFS_IOCTL_DEV_REPLACE_CMD_START;
 	ret = ioctl(fdmnt, BTRFS_IOC_DEV_REPLACE, &start_args);
 	if (do_not_background) {
-		if (ret) {
+		if (ret < 0) {
 			fprintf(stderr,
 				"ERROR: ioctl(DEV_REPLACE_START) failed on \"%s\": %s,
%s\n",
 				path, strerror(errno),
 				replace_dev_result2string(start_args.result));
 			goto leave_with_error;
+		} else if (ret > 0) {
+			fprintf(stderr,
+				"ERROR: replace start failed on %s - %s\n",
+				path, btrfs_err_str(ret));
+			goto leave_with_error;
 		}
 
 		if (start_args.result !-- 
1.7.1

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

Anand Jain

2013-Aug-30 08:50 UTC

head link

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

This fix the regression introduced by 830427d
that it no more creates the FS if disk is small
and if no mixed option is provided.
This patch will bring it to the original design
which will force mixed profile when disk is small
and go ahead to create the FS.

Which also means that before we open the device
for the write we should also check if disk is small.

v2: fixes the checkpatch.pl warnings

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

diff --git a/mkfs.c b/mkfs.c
index 73f5425..5b3b245 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -1415,6 +1415,17 @@ int main(int ac, char **av)
 	file = av[optind++];
 	ssd = is_ssd(file);
 
+	if (is_vol_small(file)) {
+		printf("SMALL VOLUME: forcing mixed metadata/data groups\n");
+		mixed = 1;
+		if (metadata_profile != data_profile) {
+			if (metadata_profile_opt || data_profile_opt) {
+				fprintf(stderr,
+	"With mixed block groups data and metadata profiles must be the
same\n");
+				exit(1);
+			}
+		}
+	}
 	/*
 	* Set default profiles according to number of added devices.
 	* For mixed groups defaults are single/single.
@@ -1435,7 +1446,6 @@ int main(int ac, char **av)
 				BTRFS_BLOCK_GROUP_RAID0 : 0; /* raid0 or single */
 		}
 	} else {
-		/* this is not needed but just for completeness */
 		metadata_profile = 0;
 		data_profile = 0;
 	}
@@ -1494,14 +1504,6 @@ int main(int ac, char **av)
 	}
 
 
-	if (mixed) {
-		if (metadata_profile != data_profile) {
-			fprintf(stderr, "With mixed block groups data and metadata "
-				"profiles must be the same\n");
-			exit(1);
-		}
-	}
-
 	blocks[0] = BTRFS_SUPER_INFO_OFFSET;
 	for (i = 1; i < 7; i++) {
 		blocks[i] = BTRFS_SUPER_INFO_OFFSET + 1024 * 1024 +
diff --git a/utils.c b/utils.c
index de182e0..b039a03 100644
--- a/utils.c
+++ b/utils.c
@@ -1964,3 +1964,32 @@ int scan_for_btrfs(int where, int update_kernel)
 	}
 	return ret;
 }
+
+int is_vol_small(char *file)
+{
+	int fd = -1;
+	int e;
+	struct stat st;
+	u64 size;
+
+	fd = open(file, O_RDONLY);
+	if (fd < 0)
+		return -errno;
+	if (fstat(fd, &st) < 0) {
+		e = -errno;
+		close(fd);
+		return e;
+	}
+	size = btrfs_device_size(fd, &st);
+	if (size == 0) {
+		close(fd);
+		return -1;
+	}
+	if (size < 1024 * 1024 * 1024) {
+		close(fd);
+		return 1;
+	} else {
+		close(fd);
+		return 0;
+	}
+}
diff --git a/utils.h b/utils.h
index 4b449b8..eb6fba3 100644
--- a/utils.h
+++ b/utils.h
@@ -82,4 +82,5 @@ 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);
 int get_btrfs_mount(const char *dev, char *mp, size_t mp_size);
+int is_vol_small(char *file);
 #endif
-- 
1.7.1

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

David Sterba

2013-Aug-30 23:22 UTC

head link

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

On Fri, Aug 30, 2013 at 04:50:37PM +0800, Anand Jain
wrote:> This fix the regression introduced by 830427d
ie. that''s
"btrfs-progs: avoid write to the disk before sure to create fs"

please note that the commit id is not stable and may change during
integration branch updates.

I was not sure if the patch is all ok and had put it into
integration-next branch (that serves me as a unstable area for things
that I''d like to look at again). The patch caused Josef some headaches
and I''ve left it out from recent integration branches until the
problems
are resolved. With the patch in this email you do so, but let me do more
testing and then I''ll integrate it.
> --- a/utils.c
> +++ b/utils.c
> @@ -1964,3 +1964,32 @@ int scan_for_btrfs(int where, int update_kernel)
> +int is_vol_small(char *file)
> +{
> +	int fd = -1;
> +	int e;
> +	struct stat st;
> +	u64 size;
> +
> +	fd = open(file, O_RDONLY);
> +	if (fd < 0)
> +		return -errno;
> +	if (fstat(fd, &st) < 0) {
> +		e = -errno;
> +		close(fd);
> +		return e;
> +	}
> +	size = btrfs_device_size(fd, &st);
> +	if (size == 0) {
> +		close(fd);
> +		return -1;
> +	}
> +	if (size < 1024 * 1024 * 1024) {
Would be good to use a properly named constant instead of the magic
number.

thanks,
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

Anand Jain

2013-Aug-31 05:29 UTC

head link

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

On 08/31/2013 07:22 AM, David Sterba wrote:> On Fri, Aug 30, 2013 at 04:50:37PM +0800, Anand Jain wrote:
>> This fix the regression introduced by 830427d
>
> ie. that''s
> "btrfs-progs: avoid write to the disk before sure to create fs"
>
> please note that the commit id is not stable and may change during
> integration branch updates.
>
> I was not sure if the patch is all ok and had put it into
> integration-next branch (that serves me as a unstable area for things
> that I''d like to look at again). The patch caused Josef some
headaches
> and I''ve left it out from recent integration branches until the
problems
> are resolved. With the patch in this email you do so, but let me do more
> testing and then I''ll integrate it.
  Thanks for taking care of this David. Let me know if anywhere
  needs more explanation.
  I have been introducing series of device related changes in
  btrfs-progs/mkfs which were/is essential bug fix. Fixes looks
  complex since originally btrfs-progs/mkfs didn''t create and
  use much of helper functions in this area, now with these
  fixes its much modular and better.

>> --- a/utils.c
>> +++ b/utils.c
>> @@ -1964,3 +1964,32 @@ int scan_for_btrfs(int where, int update_kernel)
>> +int is_vol_small(char *file)
>> +{
>> +	int fd = -1;
>> +	int e;
>> +	struct stat st;
>> +	u64 size;
>> +
>> +	fd = open(file, O_RDONLY);
>> +	if (fd < 0)
>> +		return -errno;
>> +	if (fstat(fd, &st) < 0) {
>> +		e = -errno;
>> +		close(fd);
>> +		return e;
>> +	}
>> +	size = btrfs_device_size(fd, &st);
>> +	if (size == 0) {
>> +		close(fd);
>> +		return -1;
>> +	}
>> +	if (size < 1024 * 1024 * 1024) {
>
> Would be good to use a properly named constant instead of the magic
> number.
  Yeah I notice that, but didn''t want to change from the Original.
  There are other places where this constant is needed as well,
  will spin a separate patch for that hope that''s fine.

Thanks
Anand
> thanks,
> 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
>--
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

David Sterba

2013-Sep-03 17:53 UTC

head link

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

On Sat, Aug 31, 2013 at 01:29:53PM +0800, Anand Jain
wrote:>  Thanks for taking care of this David. Let me know if anywhere
>  needs more explanation.
>  I have been introducing series of device related changes in
>  btrfs-progs/mkfs which were/is essential bug fix. Fixes looks
>  complex since originally btrfs-progs/mkfs didn''t create and
>  use much of helper functions in this area, now with these
>  fixes its much modular and better.
Yeah mkfs needs clean ups, thanks for doing that. I''m running btrfs/003
on top of various mkfs option combinations, so far it hasn''t blown up
at
mkfs time, the patches included are

btrfs-progs: mkfs should check for small vol well before
btrfs-progs: avoid write to the disk before sure to create fs

and excluded

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

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

Btrfs devel - Aug 2013 - [PATCH] btrfs-progs: mkfs should check for small vol well before

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

[PATCH] btrfs-progs: use btrfs error code for kernel errors

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

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

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

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