btrfs-progs: prevent mkfs from aborting with small volume
cover btrfs-progs: prevent mkfs from aborting with small volume
I found that mkfs.btrfs aborts when one of assigned volume is too small. Here are 2 patches to fix 2 independent problems.
Both are based on top of Chris's btrfs-progs.git:
git://git.kernel.org/pub/scm/linux/kernel/git/mason/btrfs-progs.git
Thanks,
H.Seto
Hidetoshi Seto (2):
btrfs-progs: treat reserved 1MB for superblock properly
btrfs-progs: exit if there is not enough free space for mkfs
ctree.h | 3 +++ mkfs.c | 4 ++++ volumes.c | 7 ++++++- 3 files changed, 13 insertions(+), 1 deletions(-)
PATCH 1/2 btrfs-progs: treat reserved 1MB for superblock properly
I found that mkfs.btrfs aborts when assigned multi volumes contain a small volume:
# parted /dev/sdf p Model: LSI MegaRAID SAS RMB (scsi) Disk /dev/sdf: 72.8GB Sector size (logical/physical): 512B/512B Partition Table: msdos
Number Start End Size Type File system Flags 1 32.3kB 72.4GB 72.4GB primary 2 72.4GB 72.8GB 461MB primary
# ./mkfs.btrfs -f /dev/sdf1 /dev/sdf2 : SMALL VOLUME: forcing mixed metadata/data groups adding device /dev/sdf2 id 2 mkfs.btrfs: volumes.c:852: btrfs_alloc_chunk: Assertion `!(ret)' failed. Aborted (core dumped)
This failure of btrfs_alloc_chunk was caused by following steps:
1) since there is only small space in the small device, mkfs was going to allocate a chunk from free space as much as available. So mkfs called btrfs_alloc_chunk with
size = device->total_bytes - device->used_bytes.
2) To avoid overwriting superblock, btrfs_alloc_chunk starts taking chunks at an offset of 1MB. It means that the layout of a disk will be like:
[[1MB at begging for sb][allocated chunks]* ... free space ... ]
and you can see that the available free space for allocation is:
avail = device->total_bytes - device->used_bytes - 1MB.
3) Therefore there is only free space 1MB less than requested. damn.
So this fix let mkfs know how much spaces are really there.
--- ctree.h | 3 +++ volumes.c | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletions(-)
ctree.h +3 -0
@@ -811,6 +811,9 @@ struct btrfs_csum_item {u8 csum; } __attribute__ ((__packed__)); +/* we have reserved 1M for superblock at the begging of device */ +#define BTRFS_BLOCK_RESERVED_1M_FOR_SUPER ((u64)1024 * 1024) +/* tag for the radix tree of block groups in ram */ #define BTRFS_BLOCK_GROUP_DATA (1ULL << 0)#define BTRFS_BLOCK_GROUP_SYSTEM (1ULL << 1)
volumes.c +6 -1
@@ -283,7 +283,7 @@ static int find_free_dev_extent(struct btrfs_trans_handle *trans,/* we don't want to overwrite the superblock on the drive, * so we make sure to start at an offset of at least 1MB */ - search_start =max ((u64 )1024 *1024 , search_start); + search_start =max (BTRFS_BLOCK_RESERVED_1M_FOR_SUPER , search_start);if (root->fs_info->alloc_start + num_bytes <= device->total_bytes) search_start =max (root->fs_info->alloc_start, search_start);
@@ -783,6 +783,11 @@ again:while (index < num_stripes) { device = list_entry(cur, struct btrfs_device, dev_list); avail = device->total_bytes - device->bytes_used; +/* we have reserved 1M for superblock at the head of device */ +if (avail >BTRFS_BLOCK_RESERVED_1M_FOR_SUPER ) + avail -=BTRFS_BLOCK_RESERVED_1M_FOR_SUPER ; +else + avail =0 ; cur = cur->next;if (avail >= min_free) {list_move_tail (&device->dev_list, &private_devs);
Signature · 1 line
1.7.1
PATCH 2/2 btrfs-progs: exit if there is not enough free space for mkfs
In reply to Hidetoshi SetoAgain, while playing mkfs with small volumes, I found that mkfs.btrfs aborts if there is really no spaces for a brand-new filesystem:
# ./mkfs.btrfs -f /dev/sdf1 /dev/sdf2 : SMALL VOLUME: forcing mixed metadata/data groups adding device /dev/sdf2 id 2 mkfs.btrfs: mkfs.c:184: create_one_raid_group: Assertion `!(ret)' failed. Aborted (core dumped)
This fix let mkfs prints error message if it cannot make filesystem due to a lack of free spaces.
--- mkfs.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
mkfs.c +4 -0
@@ -181,6 +181,10 @@ static int create_one_raid_group(struct btrfs_trans_handle *trans, ret =btrfs_alloc_chunk (trans, root->fs_info->extent_root, &chunk_start, &chunk_size, type); +if (ret == -ENOSPC ) { +fprintf (stderr,"not enough free space\n" ); +exit (1 ); + }BUG_ON (ret); ret = btrfs_make_block_group(trans, root->fs_info->extent_root, 0, type, BTRFS_FIRST_CHUNK_TREE_OBJECTID,
Signature · 1 line
1.7.1
Quoted 9 lines · Hidetoshi Seto
I found that mkfs.btrfs aborts when one of assigned volume is too small. Here are 2 patches to fix 2 independent problems.
Both are based on top of Chris's btrfs-progs.git:
git://git.kernel.org/pub/scm/linux/kernel/git/mason/btrfs-progs.gitThanks,
H.Seto
Thanks for looking into this - how small of a device did you test?
I tried a 2MB device w/ these 2 patches and still got:
[btrfs-progs]# truncate --size=2m testfile
[btrfs-progs]# ./mkfs.btrfs testfile
WARNING! - Btrfs v0.20-rc1-360-geeeb4e9 IS EXPERIMENTAL WARNING! - see http://btrfs.wiki.kernel.org before using
SMALL VOLUME: forcing mixed metadata/data groups
mkfs.btrfs: volumes.c:857: btrfs_alloc_chunk: Assertion `!(ret)' failed. Aborted (core dumped)
which was at:
ret = btrfs_alloc_dev_extent(trans, device,
info->chunk_root->root_key.objectid,
BTRFS_FIRST_CHUNK_TREE_OBJECTID, key.offset,
calc_size, &dev_offset);
BUG_ON(ret);:(
Also, I'm curious - I know the code existed before your patch 2/2, but I don't understand why it reserves 1MB for the first superblock when the first superblock is actually at 64k. Any idea?
-Eric
Quoted 9 lines
Hidetoshi Seto (2):
btrfs-progs: treat reserved 1MB for superblock properly
btrfs-progs: exit if there is not enough free space for mkfsctree.h | 3 +++ mkfs.c | 4 ++++ volumes.c | 7 ++++++- 3 files changed, 13 insertions(+), 1 deletions(-)
Quoted 16 lines · Eric Sandeen
Thanks for looking into this - how small of a device did you test?
I tried a 2MB device w/ these 2 patches and still got:
[btrfs-progs]# truncate --size=2m testfile
[btrfs-progs]# ./mkfs.btrfs testfileWARNING! - Btrfs v0.20-rc1-360-geeeb4e9 IS EXPERIMENTAL WARNING! - see http://btrfs.wiki.kernel.org before using
SMALL VOLUME: forcing mixed metadata/data groups
mkfs.btrfs: volumes.c:857:btrfs_alloc_chunk: Assertion `!(ret)' failed. Aborted (core dumped)which was at:
Quoted 7 lines
ret = btrfs_alloc_dev_extent(trans, device, info->chunk_root->root_key.objectid, BTRFS_FIRST_CHUNK_TREE_OBJECTID, key.offset, calc_size, &dev_offset); BUG_ON(ret);:(
Wow...
It seems that this abort is different problem from the bug which my patches are going to fix. I'll try to make new patch to fix this problem.
Quoted 6 lines
Also, I'm curious - I know the code existed before your patch 2/2, but I don't understand why it reserves 1MB for the first superblock when the first superblock is actually at 64k. Any idea?
-Eric
I'm not sure... According to the git-log, this 1M trick is in the following old commit by Chris:
commit a6de0bd778475504f42a142c83b8077993cbddfe
Author: Chris Mason <[email protected]>
Date: Thu Apr 3 16:35:48 2008 -0400
Add mirroring support across multiple drives
Thanks,
H.Seto
Quoted 37 lines · Hidetoshi Seto
Quoted 16 lines · Eric Sandeen
Thanks for looking into this - how small of a device did you test?
I tried a 2MB device w/ these 2 patches and still got:
[btrfs-progs]# truncate --size=2m testfile
[btrfs-progs]# ./mkfs.btrfs testfileWARNING! - Btrfs v0.20-rc1-360-geeeb4e9 IS EXPERIMENTAL WARNING! - see http://btrfs.wiki.kernel.org before using
SMALL VOLUME: forcing mixed metadata/data groups
mkfs.btrfs: volumes.c:857:btrfs_alloc_chunk: Assertion `!(ret)' failed. Aborted (core dumped)which was at:
Quoted 7 lines
ret = btrfs_alloc_dev_extent(trans, device, info->chunk_root->root_key.objectid, BTRFS_FIRST_CHUNK_TREE_OBJECTID, key.offset, calc_size, &dev_offset); BUG_ON(ret);:(
Wow...
It seems that this abort is different problem from the bug which my patches are going to fix. I'll try to make new patch to fix this problem.Quoted 6 lines
Also, I'm curious - I know the code existed before your patch 2/2, but I don't understand why it reserves 1MB for the first superblock when the first superblock is actually at 64k. Any idea?
-Eric
Quoted 8 lines
I'm not sure... According to the git-log, this 1M trick is in the following old commit by Chris:
commit
a6de0bd778475504f42a142c83b8077993cbddfe
Author: Chris Mason <[email protected]>
Date: Thu Apr 3 16:35:48 2008 -0400Add mirroring support across multiple drives
Yep I saw that too. Seemingly unrelated. :( Unless I'm missing something (which I probably am).
-Eric
Quoted 4 lines
Thanks,
H.Seto
Quoted 7 lines · Eric Sandeen
Quoted 7 lines
Quoted 3 lines
Also, I'm curious - I know the code existed before your patch 2/2, but I don't understand why it reserves 1MB for the first superblock when the first superblock is actually at 64k. Any idea?
I'm not sure... According to the git-log, this 1M trick is in the following old commit by Chris:
Quoted 8 lines
Quoted 5 lines
commit
a6de0bd778475504f42a142c83b8077993cbddfe
Author: Chris Mason <[email protected]>
Date: Thu Apr 3 16:35:48 2008 -0400Add mirroring support across multiple drives
Yep I saw that too. Seemingly unrelated. :( Unless I'm missing something (which I probably am).
IIRC the 1 MB of unused space (minus the first superblock) is there to avoid random overwrites from paritioners or somesuch. If the whole megabyte is overwritten including the superblock, the 1st copy lives at 64 MB which exists on practically every fs and the one at 64k can be retored from it.
david