btrfs-progs: fix mkfs.btrfs -r option
PATCH 1/2 btrfs-progs: fix mkfs.btrfs -r option
Commit 605e806166847872bb91831b397d58f95027975a broke the mkfs.btrfs -r option, because it calls make_btrfs without ever setting dev_block_count, in the -r case, so we tell it to make a filesystem of size 0.
Then we wander into ENOSPC land and segfault.
As a quick one-line-fix, just set the dev_block_count to the size of the destination image file.
---
mkfs.c +2 -0
@@ -1337,6 +1337,8 @@ int main(int ac, char **av)fprintf (stderr,"unable to zero the output file\n" );exit (1 ); } +/* our "device" is the new image file */ + dev_block_count = block_count; }if (mixed) {if (metadata_profile != data_profile) {
RFC PATCH 2/2 btrfs-progs: overhaul mkfs.btrfs -r option
The manpage for the "-r" option simply says that it will copy the path specified to -r into the newly made filesystem.
There's not a lot of reason to treat that option as differently as it is now - today it ignores discard, fs size, and mixed options, for example. It also failed to check whether the target device was mounted before proceeding. Etc...
Rework things so that we really follow the same paths whether or not -r is specified, but with one special case for -r:
* If the device does not exist, it will be created as a regular file of the minimum size to hold the -r path, or of size specified by the -b option.
This also changes a little behavior; it does not pre-fill the new file with zeros, but allows it to be sparse, and does not truncate an existing device file. If you want to start with an empty file, just don't point it at an existing file...
---
Lightly tested . .
man/mkfs.btrfs.8.in +6 -0
@@ -63,6 +63,12 @@ Specify the sectorsize, the minimum block allocation. .TP \fB\-r\fR, \fB\-\-rootdir \fIrootdir\fR Specify a directory to copy into the newly created fs. +This option is limited to a single device. As a special +case for this option, if the device does not exist, +it will be created as a regular file of either the minimum +required size, or the size specified by the +\fB\-b\fR +option. .TP \fB\-K\fR, \fB\-\-nodiscard \fR Do not perform whole device TRIM operation by default.
mkfs.c +58 -70
156 lines not on this page · raw message
utils.c +3 -0
@@ -808,6 +808,9 @@ int check_mounted(const char* file) fd =open (file,O_RDONLY );if (fd <0 ) { +/* if the file isn't there, it is clearly not mounted! */ +if (errno ==ENOENT ) +return 0 ;fprintf (stderr,"check_mounted(): Could not open %s\n" , file);return -errno; }
Re: [PATCH 2/2, RFC] btrfs-progs: overhaul mkfs.btrfs -r option
Quoted 16 lines · Eric Sandeen
The manpage for the "-r" option simply says that it will copy the path specified to -r into the newly made filesystem.
There's not a lot of reason to treat that option as differently as it is now - today it ignores discard, fs size, and mixed options, for example. It also failed to check whether the target device was mounted before proceeding. Etc...
Rework things so that we really follow the same paths whether or not -r is specified, but with one special case for -r:
* If the device does not exist, it will be created as a regular file of the minimum size to hold the -r path, or of size specified by the -b option.
Quoted 5 lines
This also changes a little behavior; it does not pre-fill the new file with zeros, but allows it to be sparse, and does not truncate an existing device file. If you want to start with an empty file, just don't point it at an existing file...
Fixes numerous bugs and I find the changes in behaviour sane and reasonable. A quick test failed for me when the image file does not exist:
$ rm image $ ./mkfs.btrfs -r . image ... not enough free space add_file_items failed unable to traverse_directory Making image is aborted. ret = -1 mkfs.btrfs: mkfs.c:1533: main: Assertion `!(ret)' failed.
seems that the estimated size is not sufficient.
'du' on the directory (without the image itself) gives me 59M, the image after failed mkfs has 102M, and it's empty when I try to mount it. Using the progs .git directory (18M) as sourcedir also failed, but it worked with man/ subdirectory (94K).
david
Re: [PATCH 2/2, RFC] btrfs-progs: overhaul mkfs.btrfs -r option
Quoted 37 lines · David Sterba
Quoted 16 lines · Eric Sandeen
The manpage for the "-r" option simply says that it will copy the path specified to -r into the newly made filesystem.
There's not a lot of reason to treat that option as differently as it is now - today it ignores discard, fs size, and mixed options, for example. It also failed to check whether the target device was mounted before proceeding. Etc...
Rework things so that we really follow the same paths whether or not -r is specified, but with one special case for -r:
* If the device does not exist, it will be created as a regular file of the minimum size to hold the -r path, or of size specified by the -b option.
Quoted 5 lines
This also changes a little behavior; it does not pre-fill the new file with zeros, but allows it to be sparse, and does not truncate an existing device file. If you want to start with an empty file, just don't point it at an existing file...
Fixes numerous bugs and I find the changes in behaviour sane and reasonable. A quick test failed for me when the image file does not exist:
$ rm image $ ./mkfs.btrfs -r . image ... not enough free space add_file_items failed unable to traverse_directory Making image is aborted. ret = -1 mkfs.btrfs: mkfs.c:1533: main: Assertion `!(ret)' failed.
Quoted 1 line
seems that the estimated size is not sufficient.
ho hum, thanks for checking. I guess I only tried it on a very small source directory. For now, figuring out btrfs space usage for a given set of files is above my pay grade and experience, I'm afraid. :(
-Eric
Quoted 7 lines
'du' on the directory (without the image itself) gives me 59M, the image after failed mkfs has 102M, and it's empty when I try to mount it. Using the progs .git directory (18M) as sourcedir also failed, but it worked with man/ subdirectory (94K).
david
Re: [PATCH 2/2, RFC] btrfs-progs: overhaul mkfs.btrfs -r option
Quoted 38 lines · Eric Sandeen
Quoted 37 lines · David Sterba
Quoted 16 lines · Eric Sandeen
The manpage for the "-r" option simply says that it will copy the path specified to -r into the newly made filesystem.
There's not a lot of reason to treat that option as differently as it is now - today it ignores discard, fs size, and mixed options, for example. It also failed to check whether the target device was mounted before proceeding. Etc...
Rework things so that we really follow the same paths whether or not -r is specified, but with one special case for -r:
* If the device does not exist, it will be created as a regular file of the minimum size to hold the -r path, or of size specified by the -b option.
Quoted 5 lines
This also changes a little behavior; it does not pre-fill the new file with zeros, but allows it to be sparse, and does not truncate an existing device file. If you want to start with an empty file, just don't point it at an existing file...
Fixes numerous bugs and I find the changes in behaviour sane and reasonable. A quick test failed for me when the image file does not exist:
$ rm image $ ./mkfs.btrfs -r . image ... not enough free space add_file_items failed unable to traverse_directory Making image is aborted. ret = -1 mkfs.btrfs: mkfs.c:1533: main: Assertion `!(ret)' failed.
Quoted 5 lines
Quoted 1 line
seems that the estimated size is not sufficient.
ho hum, thanks for checking. I guess I only tried it on a very small source directory. For now, figuring out btrfs space usage for a given set of files is above my pay grade and experience, I'm afraid. :(
We should probably make a way to compact a given FS down to minimal size. Then we don't have to worry about size estimates at all.
In other words, two passes instead of one.
-chris