Re: [PATCH] mkfs: Handle creation of filesystem larger than the first device
For the record, this fixup is needed --- a/utils.c +++ b/utils.c @@ -273,6 +273,9 @@ int make_btrfs(int fd, const char *device, const char *label, btrfs_set_item_offset(buf, btrfs_item_nr(buf, nritems), itemoff); btrfs_set_item_size(buf, btrfs_item_nr(buf, nritems), item_size); + if (num_bytes < dev_num_bytes) + dev_num_bytes = num_bytes; + dev_item = btrfs_item_ptr(buf, nritems, struct btrfs_dev_item); btrfs_set_device_id(buf, dev_item, 1); btrfs_set_device_generation(buf, dev_item, 0); --- otherwise a filesystem smaller than the block device will not report correct size of the device and this inconsistency will trigger a bug (xfstests/015) cow_file_range: 895 BUG_ON(disk_num_bytes > 896btrfs_super_total_bytes(root->fs_info->super_copy)); $ mkfs.btrfs-small-bd-big-fs -b 100M /dev/sda10 SMALL VOLUME: forcing mixed metadata/data groups WARNING! - Btrfs v0.19-190-gc1d427d IS EXPERIMENTAL WARNING! - see http://btrfs.wiki.kernel.org before using Created a data/metadata chunk of size 8388608 fs created label (null) on /dev/sda10 nodesize 4096 leafsize 4096 sectorsize 4096 size 100.00MB Btrfs v0.19-190-gc1d427d $ btrfs fi show Label: none uuid: efecdeb1-bab3-40aa-bafa-4aae1e410f28 Total devices 1 FS bytes used 28.00KB devid1 size 10.00GB used 12.00MB path /dev/sda10 ^^^ $ df -h FilesystemSize Used Avail Use% Mounted on /dev/sda10100M 28K 10G 1% /mnt/a10 ^^^ I'm writing a kernel-side patch to prevent mounting such fs, unless -o recovery is given and this will fix the fs size in superblock to sum of all device sizes. Fsck could be enhanced in the same way. david -- 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] mkfs: Handle creation of filesystem larger than the first device
On Wed 08-02-12 22:05:26, Phillip Susi wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> On 02/08/2012 06:20 PM, Jan Kara wrote:
> > Thanks for your reply. I admit I was not sure what exactly size argument
> > should be. So after looking into the code for a while I figured it should
> > be a total size of the filesystem - or differently it should be size of
> > virtual block address space in the filesystem. Thus when filesystem has
> > more devices (or admin wants to add more devices later), it can be larger
> > than the first device. But I'm not really a btrfs developper so I might be
> > wrong and of course feel free to fix the issue as you deem fit.
>
> The size of the fs is the total size of the individual disks. When you
> limit the size, you limit the size of a disk, not the whole fs. IIRC,
> mkfs initializes the fs on the first disk, which is why it was using that
> size as the size of the whole fs, and then adds the other disks after (
> which then add their size to the total fs size ).
OK, I missed that btrfs_add_to_fsid() increases total size of the
filesystem. So now I agree with you. New patch is attached. Thanks for your
review.
> It might be nice if
> mkfs could take sizes for each disk, but it only seems to take one size
> for the initial disk.
Yes, but I don't see a realistic usecase so I don't think it's really
worth the work.
Honza
--
Jan Kara
SUSE Labs, CR
>From e5f46872232520310c56327593c02ef6a7f5ea33 Mon Sep 17 00:00:00 2001
From: Jan Kara
Date: Fri, 10 Feb 2012 11:44:44 +0100
Subject: [PATCH] mkfs: Handle creation of filesystem larger than the first device
mkfs does not properly check requested size of the filesystem. Thus if the
requested size is larger than the first device, it happily creates larger
filesystem than a device it resides on which results in 'attemp to access
beyond end of device' messages from the kernel. So verify specified filesystem
size against the size of the first device.
CC: David Sterba
Signed-off-by: Jan Kara
---
mkfs.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/mkfs.c b/mkfs.c
index e3ced19..3afe9eb 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -1282,6 +1282,10 @@ int main(int ac, char **av)
ret = btrfs_prepare_device(fd, file, zero_end, &dev_block_count, &mixed);
if (block_count == 0)
block_count = dev_block_count;
+ else if (block_count > dev_block_count) {
+ fprintf(stderr, "%s is smaller than requested size\n", file);
+ exit(1);
+ }
} else {
ac = 0;
file = av[optind++];
--
1.7.1
Re: [PATCH] mkfs: Handle creation of filesystem larger than the first device
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 02/08/2012 06:20 PM, Jan Kara wrote: > Thanks for your reply. I admit I was not sure what exactly size argument > should be. So after looking into the code for a while I figured it should > be a total size of the filesystem - or differently it should be size of > virtual block address space in the filesystem. Thus when filesystem has > more devices (or admin wants to add more devices later), it can be larger > than the first device. But I'm not really a btrfs developper so I might be > wrong and of course feel free to fix the issue as you deem fit. The size of the fs is the total size of the individual disks. When you limit the size, you limit the size of a disk, not the whole fs. IIRC, mkfs initializes the fs on the first disk, which is why it was using that size as the size of the whole fs, and then adds the other disks after ( which then add their size to the total fs size ). It might be nice if mkfs could take sizes for each disk, but it only seems to take one size for the initial disk. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJPMzf2AAoJEJrBOlT6nu75Ci8H/j3+8AR5H+UGOzpwMEBmPViJ PCVc5fAqOgLlQgjAII9dF74/1a6NyC9hjWBXPlhfrc3rA0JBj6x2AknvGnTQ6/Xo 4hMu8sFSSOtHf/aTXh7B7YJ/WrqDgkiEOSpcRVJyltzhKt0bbE3t9/IfxAhvkB1z 3CuEs9UeIn9wOV2fcyXoNMWpPQ+tNkxrvE817BHjPdQ5Z1+d2Cc0AxM22lgBVsZZ J+oneFOeqSIGZ9hbr0WVEjHaWJpxEapNmVGE5RIrpneTGpe3eAijqbBa8TEg+C2R iVCT7tBG3gOGhRoApMNM2IP2TgGLHMRgwP8QQv4/9MTNrOEP3G77tbCDHBfKMNA= =g+5L -END PGP SIGNATURE- -- 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] mkfs: Handle creation of filesystem larger than the first device
On Wed 08-02-12 17:01:15, Phillip Susi wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > On 1/26/2012 11:03 AM, Jan Kara wrote: > > make_btrfs() function takes a size of filesystem as an argument. It > > uses this value to set the size of the first device as well which > > is wrong for filesystems larger than this device. It results in > > 'attemp to access beyond end of device' messages from the kernel. > > So add size of the first device as an argument to make_btrfs(). > > I don't think this patch is correct. Yes, the size switch only > applies to the first device, so it doesn't make any sense to try to > use a value larger than that device. Attempting to do so probably > should be trapped and it should error out, and the man page should > probably clarify the fact that the size is only for the first device. > > It looks like you think the size should somehow apply to multiple > devices or to the total fs size when creating on multiple devices, and > that just doesn't make sense. Thanks for your reply. I admit I was not sure what exactly size argument should be. So after looking into the code for a while I figured it should be a total size of the filesystem - or differently it should be size of virtual block address space in the filesystem. Thus when filesystem has more devices (or admin wants to add more devices later), it can be larger than the first device. But I'm not really a btrfs developper so I might be wrong and of course feel free to fix the issue as you deem fit. Honza -- Jan Kara SUSE Labs, CR -- 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] mkfs: Handle creation of filesystem larger than the first device
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 1/26/2012 11:03 AM, Jan Kara wrote: > make_btrfs() function takes a size of filesystem as an argument. It > uses this value to set the size of the first device as well which > is wrong for filesystems larger than this device. It results in > 'attemp to access beyond end of device' messages from the kernel. > So add size of the first device as an argument to make_btrfs(). I don't think this patch is correct. Yes, the size switch only applies to the first device, so it doesn't make any sense to try to use a value larger than that device. Attempting to do so probably should be trapped and it should error out, and the man page should probably clarify the fact that the size is only for the first device. It looks like you think the size should somehow apply to multiple devices or to the total fs size when creating on multiple devices, and that just doesn't make sense. -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.17 (MingW32) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJPMvCrAAoJEJrBOlT6nu75A/IH/3Pn9MFhxXI1kTu1jriA/1ZA IaCPkZbNFvS1DC5U8E75Ys4Qtn/SkwVOdGG8VCObfJKhhbWXEGKLdtllxV8WUkRM QYN3rFeb3gLxb9UIcyyRC+RDtJtVzVXClFN7WYgA2QXmCyYdnV3axzvO/tkvADuq Is28sKnYzV9poKTlghqFmEGmqcnTtfFKg9MC60wGDKEOMuAeijImGaAEp773G7+S JSOOPcuDj/Lh7ZO+duR2ul+zUI9DWr2IbZM6zUxOoN2fZEJAwJLNPsU7rBDX8w+g FVHFHrRv6wVGq0I7Dvb2flif5O0wSRA5yhK/7GanaVEMBKSV9A0c5qOE9LakL/s= =X7QW -END PGP SIGNATURE- -- 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] mkfs: Handle creation of filesystem larger than the first device
make_btrfs() function takes a size of filesystem as an argument. It uses this
value to set the size of the first device as well which is wrong for
filesystems larger than this device. It results in 'attemp to access beyond end
of device' messages from the kernel. So add size of the first device as an
argument to make_btrfs().
CC: David Sterba
Signed-off-by: Jan Kara
---
convert.c |2 +-
mkfs.c|6 --
utils.c |4 ++--
utils.h |2 +-
4 files changed, 8 insertions(+), 6 deletions(-)
As a side note, I'd guess that creating filesystem larger than all given
devices (especially in case of single device) is usually not what sysadmin
wants (we've spotted this bug when xfstest were happily creating 1 GB
filesystem on 500 MB device and it took us a while to notice the problem).
Thus maybe it should require some --force switch?
diff --git a/convert.c b/convert.c
index 291dc27..7f1932c 100644
--- a/convert.c
+++ b/convert.c
@@ -2374,7 +2374,7 @@ int do_convert(const char *devname, int datacsum, int
packing, int noxattr)
goto fail;
}
ret = make_btrfs(fd, devname, ext2_fs->super->s_volume_name,
-blocks, total_bytes, blocksize, blocksize,
+blocks, total_bytes, total_bytes, blocksize, blocksize,
blocksize, blocksize);
if (ret) {
fprintf(stderr, "unable to create initial ctree\n");
diff --git a/mkfs.c b/mkfs.c
index e3ced19..f0d29bb 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -1294,8 +1294,10 @@ int main(int ac, char **av)
first_file = file;
source_dir_size = size_sourcedir(source_dir, sectorsize,
&num_of_meta_chunks,
&size_of_data);
- if(block_count < source_dir_size)
+ if (block_count < source_dir_size)
block_count = source_dir_size;
+ dev_block_count = block_count;
+
ret = zero_output_file(fd, block_count, sectorsize);
if (ret) {
fprintf(stderr, "unable to zero the output file\n");
@@ -1321,7 +1323,7 @@ int main(int ac, char **av)
leafsize * i;
}
- ret = make_btrfs(fd, file, label, blocks, block_count,
+ ret = make_btrfs(fd, file, label, blocks, block_count, dev_block_count,
nodesize, leafsize,
sectorsize, stripesize);
if (ret) {
diff --git a/utils.c b/utils.c
index 178d1b9..f34da51 100644
--- a/utils.c
+++ b/utils.c
@@ -74,7 +74,7 @@ static u64 reference_root_table[] = {
};
int make_btrfs(int fd, const char *device, const char *label,
- u64 blocks[7], u64 num_bytes, u32 nodesize,
+ u64 blocks[7], u64 num_bytes, u64 dev_num_bytes, u32 nodesize,
u32 leafsize, u32 sectorsize, u32 stripesize)
{
struct btrfs_super_block super;
@@ -276,7 +276,7 @@ int make_btrfs(int fd, const char *device, const char
*label,
dev_item = btrfs_item_ptr(buf, nritems, struct btrfs_dev_item);
btrfs_set_device_id(buf, dev_item, 1);
btrfs_set_device_generation(buf, dev_item, 0);
- btrfs_set_device_total_bytes(buf, dev_item, num_bytes);
+ btrfs_set_device_total_bytes(buf, dev_item, dev_num_bytes);
btrfs_set_device_bytes_used(buf, dev_item,
BTRFS_MKFS_SYSTEM_GROUP_SIZE);
btrfs_set_device_io_align(buf, dev_item, sectorsize);
diff --git a/utils.h b/utils.h
index c5f55e1..bf2d5a4 100644
--- a/utils.h
+++ b/utils.h
@@ -22,7 +22,7 @@
#define BTRFS_MKFS_SYSTEM_GROUP_SIZE (4 * 1024 * 1024)
int make_btrfs(int fd, const char *device, const char *label,
- u64 blocks[6], u64 num_bytes, u32 nodesize,
+ u64 blocks[6], u64 num_bytes, u64 dev_num_bytes, u32 nodesize,
u32 leafsize, u32 sectorsize, u32 stripesize);
int btrfs_make_root_dir(struct btrfs_trans_handle *trans,
struct btrfs_root *root, u64 objectid);
--
1.7.1
--
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
