Re: [PATCH] btrfs-progs: Add minimum device size check.

2014-06-23 Thread David Sterba
On Thu, Jun 19, 2014 at 11:25:38AM +0800, Qu Wenruo wrote:
 Btrfs has global block reservation, so even mkfs.btrfs can execute
 without problem, there is still a possibility that the filesystem can't
 be mounted.
 For example when mkfs.btrfs on a 8M file on x86_64 platform, kernel will
 refuse to mount due to ENOSPC, since system block group takes 4M and
 mixed block group takes 4M, and global block reservation will takes all
 the 4M from mixed block group, which makes btrfs unable to create uuid
 tree.
 
 This patch will add minimum device size check before actually mkfs.
 The minimum size calculation uses a simplified one:
 minimum_size_for_each_dev = 2 * (system block group + global block rsv)

So the global block reserve is set to 1024 * sectorsize. I know the
minimum size guesses are easy to get wrong with various option mixes,
some sane minimum is good. A filesystem in the scale of megabytes is not
recommended anyway.

 --- a/mkfs.c
 +++ b/mkfs.c
 @@ -1337,6 +1337,15 @@ int main(int ac, char **av)
  metadata/data groups\n);
   mixed = 1;
   }
 + if (block_count  BTRFS_MIN_SIZE_EACH) {
 + fprintf(stderr,
 + Size '%llu' is too small to 
 make a usable btrfs\n,
 + block_count);
 + fprintf(stderr,
 + Recommended size for each 
 device is %llu\n,
 + BTRFS_MIN_SIZE_EACH);

I'd suggest to merge the messages into one and do not use Recommended.
As described in the changelog it's the 'minimum' accepted size.

 @@ -1370,6 +1379,21 @@ int main(int ac, char **av)
   }
   while (dev_cnt--  0) {
   file = av[optind++];
 + ret = test_minimum_size(file);
 + if (ret  0) {
 + fprintf(stderr, Failed to check size for '%s': %s\n,
 + file, strerror(-ret));
 + exit(1);
 + }
 + if (ret  0) {
 + fprintf(stderr,
 + '%s' is too small to make a usable btrfs\n,
 + file);
 + fprintf(stderr,
 + Recommended size for each device is %llu\n,
 + BTRFS_MIN_SIZE_EACH);
 + exit(1);

Same here.

 + }
   if (is_block_device(file))
   if (test_dev_for_mkfs(file, force_overwrite, estr)) {
   fprintf(stderr, Error: %s, estr);
 --- a/utils.h
 +++ b/utils.h
 @@ -101,5 +101,18 @@ int get_btrfs_mount(const char *dev, char *mp, size_t 
 mp_size);
  int find_mount_root(const char *path, char **mount_root);
  int get_device_info(int fd, u64 devid,
   struct btrfs_ioctl_dev_info_args *di_args);
 +int test_minimum_size(const char *file);
  
 +/*
 + * Btrfs minimum size calculation is complicated, it should include at least:
 + * 1. system group size
 + * 2. minimum global block reserve
 + * 3. metadata used at mkfs
 + * 4. space reservation to create uuid for first mount.
 + * Also, raid factor should also be taken into consideration.
 + * To avoid the overkill calculation, (system group + global block rsv) * 2
 + * for *EACH* device should be good enough.
 + */
 +#define BTRFS_MIN_SIZE_EACH  (2 * (BTRFS_MKFS_SYSTEM_GROUP_SIZE + \
 +   ((u64)sysconf(_SC_PAGESIZE)  10)))

The use of PAGESIZE here is well hidden in the macro, although it should
be sectorsize of the currently created filesystem. Also,
BTRFS_MIN_SIZE_EACH should be rather denote it's related to a device, so
BTRFS_MIN_DEVICE_SIZE.

Separating the minimum global block reserve size into a macro would be
also cleaner.

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


Re: [PATCH] btrfs-progs: Add minimum device size check.

2014-06-23 Thread Qu Wenruo


 Original Message 
Subject: Re: [PATCH] btrfs-progs: Add minimum device size check.
From: David Sterba dste...@suse.cz
To: Qu Wenruo quwen...@cn.fujitsu.com
Date: 2014年06月23日 22:36

On Thu, Jun 19, 2014 at 11:25:38AM +0800, Qu Wenruo wrote:

Btrfs has global block reservation, so even mkfs.btrfs can execute
without problem, there is still a possibility that the filesystem can't
be mounted.
For example when mkfs.btrfs on a 8M file on x86_64 platform, kernel will
refuse to mount due to ENOSPC, since system block group takes 4M and
mixed block group takes 4M, and global block reservation will takes all
the 4M from mixed block group, which makes btrfs unable to create uuid
tree.

This patch will add minimum device size check before actually mkfs.
The minimum size calculation uses a simplified one:
minimum_size_for_each_dev = 2 * (system block group + global block rsv)

So the global block reserve is set to 1024 * sectorsize. I know the
minimum size guesses are easy to get wrong with various option mixes,
some sane minimum is good. A filesystem in the scale of megabytes is not
recommended anyway.


--- a/mkfs.c
+++ b/mkfs.c
@@ -1337,6 +1337,15 @@ int main(int ac, char **av)
   metadata/data groups\n);
mixed = 1;
}
+   if (block_count  BTRFS_MIN_SIZE_EACH) {
+   fprintf(stderr,
+   Size '%llu' is too small to make a 
usable btrfs\n,
+   block_count);
+   fprintf(stderr,
+   Recommended size for each device is 
%llu\n,
+   BTRFS_MIN_SIZE_EACH);

I'd suggest to merge the messages into one and do not use Recommended.
As described in the changelog it's the 'minimum' accepted size.


@@ -1370,6 +1379,21 @@ int main(int ac, char **av)
}
while (dev_cnt--  0) {
file = av[optind++];
+   ret = test_minimum_size(file);
+   if (ret  0) {
+   fprintf(stderr, Failed to check size for '%s': %s\n,
+   file, strerror(-ret));
+   exit(1);
+   }
+   if (ret  0) {
+   fprintf(stderr,
+   '%s' is too small to make a usable btrfs\n,
+   file);
+   fprintf(stderr,
+   Recommended size for each device is %llu\n,
+   BTRFS_MIN_SIZE_EACH);
+   exit(1);

Same here.


+   }
if (is_block_device(file))
if (test_dev_for_mkfs(file, force_overwrite, estr)) {
fprintf(stderr, Error: %s, estr);
--- a/utils.h
+++ b/utils.h
@@ -101,5 +101,18 @@ int get_btrfs_mount(const char *dev, char *mp, size_t 
mp_size);
  int find_mount_root(const char *path, char **mount_root);
  int get_device_info(int fd, u64 devid,
struct btrfs_ioctl_dev_info_args *di_args);
+int test_minimum_size(const char *file);
  
+/*

+ * Btrfs minimum size calculation is complicated, it should include at least:
+ * 1. system group size
+ * 2. minimum global block reserve
+ * 3. metadata used at mkfs
+ * 4. space reservation to create uuid for first mount.
+ * Also, raid factor should also be taken into consideration.
+ * To avoid the overkill calculation, (system group + global block rsv) * 2
+ * for *EACH* device should be good enough.
+ */
+#define BTRFS_MIN_SIZE_EACH(2 * (BTRFS_MKFS_SYSTEM_GROUP_SIZE + \
+ ((u64)sysconf(_SC_PAGESIZE)  10)))

The use of PAGESIZE here is well hidden in the macro, although it should
be sectorsize of the currently created filesystem. Also,
BTRFS_MIN_SIZE_EACH should be rather denote it's related to a device, so
BTRFS_MIN_DEVICE_SIZE.

Separating the minimum global block reserve size into a macro would be
also cleaner.

Thanks.

Thanks for the comment, I'll send the v3 patch fixing all the problem.

Thanks,
Qu
--
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


[PATCH] btrfs-progs: Add minimum device size check.

2014-06-18 Thread Qu Wenruo
Btrfs has global block reservation, so even mkfs.btrfs can execute
without problem, there is still a possibility that the filesystem can't
be mounted.
For example when mkfs.btrfs on a 8M file on x86_64 platform, kernel will
refuse to mount due to ENOSPC, since system block group takes 4M and
mixed block group takes 4M, and global block reservation will takes all
the 4M from mixed block group, which makes btrfs unable to create uuid
tree.

This patch will add minimum device size check before actually mkfs.
The minimum size calculation uses a simplified one:
minimum_size_for_each_dev = 2 * (system block group + global block rsv)

Signed-off-by: Qu Wenruo quwen...@cn.fujitsu.com
---
 mkfs.c  | 24 
 utils.c | 30 ++
 utils.h | 13 +
 3 files changed, 67 insertions(+)

diff --git a/mkfs.c b/mkfs.c
index 16e9222..58c751c 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -1337,6 +1337,15 @@ int main(int ac, char **av)
   metadata/data groups\n);
mixed = 1;
}
+   if (block_count  BTRFS_MIN_SIZE_EACH) {
+   fprintf(stderr,
+   Size '%llu' is too small to 
make a usable btrfs\n,
+   block_count);
+   fprintf(stderr,
+   Recommended size for each 
device is %llu\n,
+   BTRFS_MIN_SIZE_EACH);
+   exit(1);
+   }
zero_end = 0;
break;
case 'V':
@@ -1370,6 +1379,21 @@ int main(int ac, char **av)
}
while (dev_cnt--  0) {
file = av[optind++];
+   ret = test_minimum_size(file);
+   if (ret  0) {
+   fprintf(stderr, Failed to check size for '%s': %s\n,
+   file, strerror(-ret));
+   exit(1);
+   }
+   if (ret  0) {
+   fprintf(stderr,
+   '%s' is too small to make a usable btrfs\n,
+   file);
+   fprintf(stderr,
+   Recommended size for each device is %llu\n,
+   BTRFS_MIN_SIZE_EACH);
+   exit(1);
+   }
if (is_block_device(file))
if (test_dev_for_mkfs(file, force_overwrite, estr)) {
fprintf(stderr, Error: %s, estr);
diff --git a/utils.c b/utils.c
index e130849..6f47acc 100644
--- a/utils.c
+++ b/utils.c
@@ -719,6 +719,16 @@ int is_block_device(const char *path)
return S_ISBLK(statbuf.st_mode);
 }
 
+int is_regular_file(const char *path)
+{
+   struct stat statbuf;
+
+   if (stat(path, statbuf)  0)
+   return -errno;
+
+   return S_ISREG(statbuf.st_mode);
+}
+
 /*
  * check if given path is a mount point
  * return 1 if yes. 0 if no. -1 for error
@@ -2206,3 +2216,23 @@ int find_mount_root(const char *path, char **mount_root)
free(longest_match);
return ret;
 }
+
+int test_minimum_size(const char *file)
+{
+   int fd;
+   struct stat statbuf;
+
+   fd = open(file, O_RDONLY);
+   if (fd  0)
+   return -errno;
+   if (stat(file, statbuf)  0) {
+   close(fd);
+   return -errno;
+   }
+   if (btrfs_device_size(fd, statbuf)  BTRFS_MIN_SIZE_EACH) {
+   close(fd);
+   return 1;
+   }
+   close(fd);
+   return 0;
+}
diff --git a/utils.h b/utils.h
index db8d63c..7f30d12 100644
--- a/utils.h
+++ b/utils.h
@@ -101,5 +101,18 @@ int get_btrfs_mount(const char *dev, char *mp, size_t 
mp_size);
 int find_mount_root(const char *path, char **mount_root);
 int get_device_info(int fd, u64 devid,
struct btrfs_ioctl_dev_info_args *di_args);
+int test_minimum_size(const char *file);
 
+/*
+ * Btrfs minimum size calculation is complicated, it should include at least:
+ * 1. system group size
+ * 2. minimum global block reserve
+ * 3. metadata used at mkfs
+ * 4. space reservation to create uuid for first mount.
+ * Also, raid factor should also be taken into consideration.
+ * To avoid the overkill calculation, (system group + global block rsv) * 2
+ * for *EACH* device should be good enough.
+ */
+#define BTRFS_MIN_SIZE_EACH(2 * (BTRFS_MKFS_SYSTEM_GROUP_SIZE + \
+ ((u64)sysconf(_SC_PAGESIZE)  10)))
 #endif
-- 
2.0.0

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

[PATCH] btrfs-progs: Add minimum device size check.

2014-06-18 Thread Qu Wenruo
Btrfs has global block reservation, so even mkfs.btrfs can execute
without problem, there is still a possibility that the filesystem can't
be mounted.
For example when mkfs.btrfs on a 8M file on x86_64 platform, kernel will
refuse to mount due to ENOSPC, since system block group takes 4M and
mixed block group takes 4M, and global block reservation will takes all
the 4M from mixed block group, which makes btrfs unable to create uuid
tree.

This patch will add minimum device size check before actually mkfs.
The minimum size calculation uses a simplified one:
minimum_size_for_each_dev = 2 * (system block group + global block rsv)

Signed-off-by: Qu Wenruo quwen...@cn.fujitsu.com
---
 mkfs.c  | 24 
 utils.c | 30 ++
 utils.h | 13 +
 3 files changed, 67 insertions(+)

diff --git a/mkfs.c b/mkfs.c
index 16e9222..58c751c 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -1337,6 +1337,15 @@ int main(int ac, char **av)
   metadata/data groups\n);
mixed = 1;
}
+   if (block_count  BTRFS_MIN_SIZE_EACH) {
+   fprintf(stderr,
+   Size '%llu' is too small to 
make a usable btrfs\n,
+   block_count);
+   fprintf(stderr,
+   Recommended size for each 
device is %llu\n,
+   BTRFS_MIN_SIZE_EACH);
+   exit(1);
+   }
zero_end = 0;
break;
case 'V':
@@ -1370,6 +1379,21 @@ int main(int ac, char **av)
}
while (dev_cnt--  0) {
file = av[optind++];
+   ret = test_minimum_size(file);
+   if (ret  0) {
+   fprintf(stderr, Failed to check size for '%s': %s\n,
+   file, strerror(-ret));
+   exit(1);
+   }
+   if (ret  0) {
+   fprintf(stderr,
+   '%s' is too small to make a usable btrfs\n,
+   file);
+   fprintf(stderr,
+   Recommended size for each device is %llu\n,
+   BTRFS_MIN_SIZE_EACH);
+   exit(1);
+   }
if (is_block_device(file))
if (test_dev_for_mkfs(file, force_overwrite, estr)) {
fprintf(stderr, Error: %s, estr);
diff --git a/utils.c b/utils.c
index e130849..6f47acc 100644
--- a/utils.c
+++ b/utils.c
@@ -719,6 +719,16 @@ int is_block_device(const char *path)
return S_ISBLK(statbuf.st_mode);
 }
 
+int is_regular_file(const char *path)
+{
+   struct stat statbuf;
+
+   if (stat(path, statbuf)  0)
+   return -errno;
+
+   return S_ISREG(statbuf.st_mode);
+}
+
 /*
  * check if given path is a mount point
  * return 1 if yes. 0 if no. -1 for error
@@ -2206,3 +2216,23 @@ int find_mount_root(const char *path, char **mount_root)
free(longest_match);
return ret;
 }
+
+int test_minimum_size(const char *file)
+{
+   int fd;
+   struct stat statbuf;
+
+   fd = open(file, O_RDONLY);
+   if (fd  0)
+   return -errno;
+   if (stat(file, statbuf)  0) {
+   close(fd);
+   return -errno;
+   }
+   if (btrfs_device_size(fd, statbuf)  BTRFS_MIN_SIZE_EACH) {
+   close(fd);
+   return 1;
+   }
+   close(fd);
+   return 0;
+}
diff --git a/utils.h b/utils.h
index db8d63c..7f30d12 100644
--- a/utils.h
+++ b/utils.h
@@ -101,5 +101,18 @@ int get_btrfs_mount(const char *dev, char *mp, size_t 
mp_size);
 int find_mount_root(const char *path, char **mount_root);
 int get_device_info(int fd, u64 devid,
struct btrfs_ioctl_dev_info_args *di_args);
+int test_minimum_size(const char *file);
 
+/*
+ * Btrfs minimum size calculation is complicated, it should include at least:
+ * 1. system group size
+ * 2. minimum global block reserve
+ * 3. metadata used at mkfs
+ * 4. space reservation to create uuid for first mount.
+ * Also, raid factor should also be taken into consideration.
+ * To avoid the overkill calculation, (system group + global block rsv) * 2
+ * for *EACH* device should be good enough.
+ */
+#define BTRFS_MIN_SIZE_EACH(2 * (BTRFS_MKFS_SYSTEM_GROUP_SIZE + \
+ ((u64)sysconf(_SC_PAGESIZE)  10)))
 #endif
-- 
2.0.0

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