diff mbox

mkfs.btrfs: allow UUID specification at mkfs time

Message ID 5372C457.3000706@redhat.com
State Superseded, archived
Delegated to: David Sterba
Headers show

Commit Message

Eric Sandeen May 14, 2014, 1:18 a.m. UTC
Allow the specification of the filesystem UUID at mkfs time.

(Implemented only for mkfs.btrfs, not btrfs-convert).

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---



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

Comments

Wang Shilong May 14, 2014, 7:31 a.m. UTC | #1
On 05/14/2014 09:18 AM, Eric Sandeen wrote:
> Allow the specification of the filesystem UUID at mkfs time.
>
> (Implemented only for mkfs.btrfs, not btrfs-convert).
Just out of curiosity, this option is used for what kind of use case?
I notice Ext4 also has this option.:-)

>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> diff --git a/btrfs-convert.c b/btrfs-convert.c
> index a8b2c51..d62d4f8 100644
> --- a/btrfs-convert.c
> +++ b/btrfs-convert.c
> @@ -2240,7 +2240,7 @@ static int do_convert(const char *devname, int datacsum, int packing,
>   		goto fail;
>   	}
>   	ret = make_btrfs(fd, devname, ext2_fs->super->s_volume_name,
> -			 blocks, total_bytes, blocksize, blocksize,
> +			 NULL, blocks, total_bytes, blocksize, blocksize,
>   			 blocksize, blocksize, 0);
>   	if (ret) {
>   		fprintf(stderr, "unable to create initial ctree: %s\n",
> diff --git a/man/mkfs.btrfs.8.in b/man/mkfs.btrfs.8.in
> index bef509d..4450d03 100644
> --- a/man/mkfs.btrfs.8.in
> +++ b/man/mkfs.btrfs.8.in
> @@ -16,6 +16,7 @@ mkfs.btrfs \- create a btrfs filesystem
>   [ \fB\-r\fP\fI rootdir\fP ]
>   [ \fB\-K\fP ]
>   [ \fB\-O\fP\fI feature1,feature2,...\fP ]
> +[ \fB\-U\fP\fI uuid\fP ]
>   [ \fB\-h\fP ]
>   [ \fB\-V\fP ]
>   \fI device\fP [ \fIdevice ...\fP ]
> @@ -90,6 +91,9 @@ To see all run
>   
>   \fBmkfs.btrfs -O list-all\fR
>   .TP
> +\fB\-U\fR, \fB\-\-uuid \fR
> +Create the filesystem with the specified UUID.
> +.TP
>   \fB\-V\fR, \fB\-\-version\fR
>   Print the \fBmkfs.btrfs\fP version and exit.
>   .SH UNIT
> diff --git a/mkfs.c b/mkfs.c
> index dbd83f5..eccb08f 100644
> --- a/mkfs.c
> +++ b/mkfs.c
> @@ -288,6 +288,7 @@ static void print_usage(void)
>   	fprintf(stderr, "\t -r --rootdir the source directory\n");
>   	fprintf(stderr, "\t -K --nodiscard do not perform whole device TRIM\n");
>   	fprintf(stderr, "\t -O --features comma separated list of filesystem features\n");
> +	fprintf(stderr, "\t -U --uuid specify the filesystem UUID\n");
>   	fprintf(stderr, "\t -V --version print the mkfs.btrfs version and exit\n");
>   	fprintf(stderr, "%s\n", BTRFS_BUILD_VERSION);
>   	exit(1);
> @@ -351,6 +352,7 @@ static struct option long_options[] = {
>   	{ "rootdir", 1, NULL, 'r' },
>   	{ "nodiscard", 0, NULL, 'K' },
>   	{ "features", 0, NULL, 'O' },
> +	{ "uuid", 0, NULL, 'U' },
>   	{ NULL, 0, NULL, 0}
>   };
>   
> @@ -1273,11 +1275,12 @@ int main(int ac, char **av)
>   	int dev_cnt = 0;
>   	int saved_optind;
>   	char estr[100];
> +	char *fs_uuid = NULL;
>   	u64 features = DEFAULT_MKFS_FEATURES;
>   
>   	while(1) {
>   		int c;
> -		c = getopt_long(ac, av, "A:b:fl:n:s:m:d:L:O:r:VMK",
> +		c = getopt_long(ac, av, "A:b:fl:n:s:m:d:L:O:r:U:VMK",
>   				long_options, &option_index);
>   		if (c < 0)
>   			break;
> @@ -1346,6 +1349,9 @@ int main(int ac, char **av)
>   				source_dir = optarg;
>   				source_dir_set = 1;
>   				break;
> +			case 'U':
> +				fs_uuid = optarg;
> +				break;
>   			case 'K':
>   				discard = 0;
>   				break;
> @@ -1514,7 +1520,7 @@ int main(int ac, char **av)
>   
>   	process_fs_features(features);
>   
> -	ret = make_btrfs(fd, file, label, blocks, dev_block_count,
> +	ret = make_btrfs(fd, file, label, fs_uuid, blocks, dev_block_count,
>   			 nodesize, leafsize,
>   			 sectorsize, stripesize, features);
>   	if (ret) {
> diff --git a/utils.c b/utils.c
> index 3e9c527..3f46e8f 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -93,12 +93,12 @@ static u64 reference_root_table[] = {
>   	[6] =	BTRFS_CSUM_TREE_OBJECTID,
>   };
>   
> -int make_btrfs(int fd, const char *device, const char *label,
> +int make_btrfs(int fd, const char *device, const char *label, char *fs_uuid,
>   	       u64 blocks[7], u64 num_bytes, u32 nodesize,
>   	       u32 leafsize, u32 sectorsize, u32 stripesize, u64 features)
>   {
>   	struct btrfs_super_block super;
> -	struct extent_buffer *buf;
> +	struct extent_buffer *buf = NULL;
>   	struct btrfs_root_item root_item;
>   	struct btrfs_disk_key disk_key;
>   	struct btrfs_extent_item *extent_item;
> @@ -125,7 +125,14 @@ int make_btrfs(int fd, const char *device, const char *label,
>   	memset(&super, 0, sizeof(super));
>   
>   	num_bytes = (num_bytes / sectorsize) * sectorsize;
> -	uuid_generate(super.fsid);
> +	if (fs_uuid) {
> +		if (uuid_parse(fs_uuid, super.fsid) != 0) {
> +			fprintf(stderr, "could not parse UUID: %s\n", fs_uuid);
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +	} else
> +		uuid_generate(super.fsid);
>   	uuid_generate(super.dev_item.uuid);
>   	uuid_generate(chunk_tree_uuid);
>   
> diff --git a/utils.h b/utils.h
> index 3c62066..054cbb6 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -40,7 +40,7 @@
>   #define BTRFS_UUID_UNPARSED_SIZE	37
>   
>   int make_btrfs(int fd, const char *device, const char *label,
> -	       u64 blocks[6], u64 num_bytes, u32 nodesize,
> +	       char *fs_uuid, u64 blocks[6], u64 num_bytes, u32 nodesize,
>   	       u32 leafsize, u32 sectorsize, u32 stripesize, u64 features);
>   int btrfs_make_root_dir(struct btrfs_trans_handle *trans,
>   			struct btrfs_root *root, u64 objectid);
>
>
> --
> 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
Brendan Hide May 14, 2014, 12:25 p.m. UTC | #2
On 14/05/14 09:31, Wang Shilong wrote:
> On 05/14/2014 09:18 AM, Eric Sandeen wrote:
>> Allow the specification of the filesystem UUID at mkfs time.
>>
>> (Implemented only for mkfs.btrfs, not btrfs-convert).
> Just out of curiosity, this option is used for what kind of use case?
> I notice Ext4 also has this option.:-)
Personally I can't think of any "average" or "normal" use case. The 
simplest case however is in using predictable/predetermined UUIDs.

Certain things, such as testing or perhaps even large-scale automation, 
are likely simpler to implement with a predictable UUID.
Eric Sandeen May 14, 2014, 1:28 p.m. UTC | #3
On 5/14/14, 2:31 AM, Wang Shilong wrote:
> On 05/14/2014 09:18 AM, Eric Sandeen wrote:
>> Allow the specification of the filesystem UUID at mkfs time.
>>
>> (Implemented only for mkfs.btrfs, not btrfs-convert).
> Just out of curiosity, this option is used for what kind of use case?
> I notice Ext4 also has this option.:-)

I don't actually have any use case myself, but a fedora bug filer
was begging for it.  I'm not sure of their usecase.

And you are right, ext4 has the option as well.

Changing the UUID after mkfs in btrfs would be very hard, but
at mkfs time it's trivial.

-Eric

>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/btrfs-convert.c b/btrfs-convert.c
>> index a8b2c51..d62d4f8 100644
>> --- a/btrfs-convert.c
>> +++ b/btrfs-convert.c
>> @@ -2240,7 +2240,7 @@ static int do_convert(const char *devname, int datacsum, int packing,
>>           goto fail;
>>       }
>>       ret = make_btrfs(fd, devname, ext2_fs->super->s_volume_name,
>> -             blocks, total_bytes, blocksize, blocksize,
>> +             NULL, blocks, total_bytes, blocksize, blocksize,
>>                blocksize, blocksize, 0);
>>       if (ret) {
>>           fprintf(stderr, "unable to create initial ctree: %s\n",
>> diff --git a/man/mkfs.btrfs.8.in b/man/mkfs.btrfs.8.in
>> index bef509d..4450d03 100644
>> --- a/man/mkfs.btrfs.8.in
>> +++ b/man/mkfs.btrfs.8.in
>> @@ -16,6 +16,7 @@ mkfs.btrfs \- create a btrfs filesystem
>>   [ \fB\-r\fP\fI rootdir\fP ]
>>   [ \fB\-K\fP ]
>>   [ \fB\-O\fP\fI feature1,feature2,...\fP ]
>> +[ \fB\-U\fP\fI uuid\fP ]
>>   [ \fB\-h\fP ]
>>   [ \fB\-V\fP ]
>>   \fI device\fP [ \fIdevice ...\fP ]
>> @@ -90,6 +91,9 @@ To see all run
>>     \fBmkfs.btrfs -O list-all\fR
>>   .TP
>> +\fB\-U\fR, \fB\-\-uuid \fR
>> +Create the filesystem with the specified UUID.
>> +.TP
>>   \fB\-V\fR, \fB\-\-version\fR
>>   Print the \fBmkfs.btrfs\fP version and exit.
>>   .SH UNIT
>> diff --git a/mkfs.c b/mkfs.c
>> index dbd83f5..eccb08f 100644
>> --- a/mkfs.c
>> +++ b/mkfs.c
>> @@ -288,6 +288,7 @@ static void print_usage(void)
>>       fprintf(stderr, "\t -r --rootdir the source directory\n");
>>       fprintf(stderr, "\t -K --nodiscard do not perform whole device TRIM\n");
>>       fprintf(stderr, "\t -O --features comma separated list of filesystem features\n");
>> +    fprintf(stderr, "\t -U --uuid specify the filesystem UUID\n");
>>       fprintf(stderr, "\t -V --version print the mkfs.btrfs version and exit\n");
>>       fprintf(stderr, "%s\n", BTRFS_BUILD_VERSION);
>>       exit(1);
>> @@ -351,6 +352,7 @@ static struct option long_options[] = {
>>       { "rootdir", 1, NULL, 'r' },
>>       { "nodiscard", 0, NULL, 'K' },
>>       { "features", 0, NULL, 'O' },
>> +    { "uuid", 0, NULL, 'U' },
>>       { NULL, 0, NULL, 0}
>>   };
>>   @@ -1273,11 +1275,12 @@ int main(int ac, char **av)
>>       int dev_cnt = 0;
>>       int saved_optind;
>>       char estr[100];
>> +    char *fs_uuid = NULL;
>>       u64 features = DEFAULT_MKFS_FEATURES;
>>         while(1) {
>>           int c;
>> -        c = getopt_long(ac, av, "A:b:fl:n:s:m:d:L:O:r:VMK",
>> +        c = getopt_long(ac, av, "A:b:fl:n:s:m:d:L:O:r:U:VMK",
>>                   long_options, &option_index);
>>           if (c < 0)
>>               break;
>> @@ -1346,6 +1349,9 @@ int main(int ac, char **av)
>>                   source_dir = optarg;
>>                   source_dir_set = 1;
>>                   break;
>> +            case 'U':
>> +                fs_uuid = optarg;
>> +                break;
>>               case 'K':
>>                   discard = 0;
>>                   break;
>> @@ -1514,7 +1520,7 @@ int main(int ac, char **av)
>>         process_fs_features(features);
>>   -    ret = make_btrfs(fd, file, label, blocks, dev_block_count,
>> +    ret = make_btrfs(fd, file, label, fs_uuid, blocks, dev_block_count,
>>                nodesize, leafsize,
>>                sectorsize, stripesize, features);
>>       if (ret) {
>> diff --git a/utils.c b/utils.c
>> index 3e9c527..3f46e8f 100644
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -93,12 +93,12 @@ static u64 reference_root_table[] = {
>>       [6] =    BTRFS_CSUM_TREE_OBJECTID,
>>   };
>>   -int make_btrfs(int fd, const char *device, const char *label,
>> +int make_btrfs(int fd, const char *device, const char *label, char *fs_uuid,
>>              u64 blocks[7], u64 num_bytes, u32 nodesize,
>>              u32 leafsize, u32 sectorsize, u32 stripesize, u64 features)
>>   {
>>       struct btrfs_super_block super;
>> -    struct extent_buffer *buf;
>> +    struct extent_buffer *buf = NULL;
>>       struct btrfs_root_item root_item;
>>       struct btrfs_disk_key disk_key;
>>       struct btrfs_extent_item *extent_item;
>> @@ -125,7 +125,14 @@ int make_btrfs(int fd, const char *device, const char *label,
>>       memset(&super, 0, sizeof(super));
>>         num_bytes = (num_bytes / sectorsize) * sectorsize;
>> -    uuid_generate(super.fsid);
>> +    if (fs_uuid) {
>> +        if (uuid_parse(fs_uuid, super.fsid) != 0) {
>> +            fprintf(stderr, "could not parse UUID: %s\n", fs_uuid);
>> +            ret = -EINVAL;
>> +            goto out;
>> +        }
>> +    } else
>> +        uuid_generate(super.fsid);
>>       uuid_generate(super.dev_item.uuid);
>>       uuid_generate(chunk_tree_uuid);
>>   diff --git a/utils.h b/utils.h
>> index 3c62066..054cbb6 100644
>> --- a/utils.h
>> +++ b/utils.h
>> @@ -40,7 +40,7 @@
>>   #define BTRFS_UUID_UNPARSED_SIZE    37
>>     int make_btrfs(int fd, const char *device, const char *label,
>> -           u64 blocks[6], u64 num_bytes, u32 nodesize,
>> +           char *fs_uuid, u64 blocks[6], u64 num_bytes, u32 nodesize,
>>              u32 leafsize, u32 sectorsize, u32 stripesize, u64 features);
>>   int btrfs_make_root_dir(struct btrfs_trans_handle *trans,
>>               struct btrfs_root *root, u64 objectid);
>>
>>
>> -- 
>> 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 Pottage May 14, 2014, 1:34 p.m. UTC | #4
On 14/05/14 08:31, Wang Shilong wrote:
> On 05/14/2014 09:18 AM, Eric Sandeen wrote:
>> Allow the specification of the filesystem UUID at mkfs time.
>>
>> (Implemented only for mkfs.btrfs, not btrfs-convert).
> Just out of curiosity, this option is used for what kind of use case?
> I notice Ext4 also has this option.:-)
I have used it a few times when replacing the hard disc of a Linux 
system, while trying to leave everything else untouched.

Many distros, including Debian and Ubuntu write the /etc/fstab to 
specify volumes by UUID instead of by label or device path.

I have also had the misfortune to use an embedded system where the boot 
volume UUID was configured into the flash in a non obvious way, so the 
easiest fix was to set-up the boot and root volumes on the replacement 
hard disc to have the same UUID.

Of course in either case I could have just taken a bit for bit copy of 
the source volume using dd, but that had it'd own problems because the 
destination was smaller. I also wanted to defrag the fs while copying it.
Duncan May 14, 2014, 1:34 p.m. UTC | #5
Brendan Hide posted on Wed, 14 May 2014 14:25:22 +0200 as excerpted:

> On 14/05/14 09:31, Wang Shilong wrote:
>> On 05/14/2014 09:18 AM, Eric Sandeen wrote:
>>> Allow the specification of the filesystem UUID at mkfs time.
>>>
>>> (Implemented only for mkfs.btrfs, not btrfs-convert).
>> Just out of curiosity, this option is used for what kind of use case?
>> I notice Ext4 also has this option.:-)

> Personally I can't think of any "average" or "normal" use case. The
> simplest case however is in using predictable/predetermined UUIDs.

AFAIK the most common use-case would be when redoing filesystems already 
listed in fstab with UUID= mounting.

I use and prefer LABEL= instead of UUID= mounting here, but I commonly 
keep a working and at least one identically sized partition backup 
filesystem copy of all non-throw-away filesystems, with fstab entries for 
both the working and backup versions, and periodically do a mkfs and 
recopy of the backup, with occasional boots to the backup and mkfs and 
recopy of the working version as well.  As I use LABEL= fstab entries I 
ensure that I specify the same label at mkfs time so I don't have to redo 
the fstab, and people that use UUID= fstab entries would find the ability 
to specify UUID at mkfs time as useful as I do the ability to specify 
label. =:^)

Many grub2 configurations also uses UUID so the same idea applies there.
Goffredo Baroncelli May 14, 2014, 2:39 p.m. UTC | #6
Hi Eric,

On 05/14/2014 03:18 AM, Eric Sandeen wrote:
> Allow the specification of the filesystem UUID at mkfs time.

I suggest to add some warning when this options is used, because the behavior could be very different than the one expected.

I suspect that BTRFS tracks the filesystem by UUID and not by devices. When two filesystems have the same UUID at the same time, it may mount the wrong one.

$ #
$ # Make two *different* filesystems with the *same* UUID
$ #
$ UUID=e285c9bd-ea97-40b3-ad7d-8713dcfd5eea
$ sudo ./mkfs.btrfs -f -U $UUID /dev/vdg
$ sudo ./mkfs.btrfs -f -U $UUID /dev/vdh

$ #
$ # from the beginning "btrfs fi show" reports wrong information
$ #
$ sudo btrfs fi show
Label: none  uuid: e285c9bd-ea97-40b3-ad7d-8713dcfd5eea
	Total devices 1 FS bytes used 96.00KB
	devid    1 size 50.00GB used 4.00MB path /dev/vdh
	devid    1 size 50.00GB used 2.04GB path /dev/vdg

$ #
$ # mount the first one, create a new file then un-mount it
$ #
$ sudo mount /dev/vdg /mnt/btrfs1
$ sudo touch /mnt/btrfs1/dev-vdg
$ sudo umount /dev/vdg

$ #
$ # mount the second one, it should be empty
$ # instead btrfs mount the first one
$ #
$ sudo mount /dev/vdh /mnt/btrfs2
$ ls -l /mnt/btrfs2
total 0
-rw-r--r-- 1 root root 0 May 14 16:12 dev-vdg


I am not against this option; I am suggesting to add a explicit warning to the user about the risk of doing that, both on the man pages and into the program. 
The warning should say that this option is only for testing. Better ask for a confirmation (even with an undocumented switch like '--I-know-that-I-am-doing-something-really-dangerous').

For the record, BTRFS seems unable to mount at the same time two different filesystems with the same UUID:

$ #
$ # try to mount two fs with the same UUID, but BTRFS doesn't allow it
$ #
$ sudo mount /dev/vdh /mnt/btrfs2
$ sudo mount /dev/vdg /mnt/btrfs1
ERROR: mount failed : 16 - Device or resource busy


BR
G.Baroncelli
Eric Sandeen May 14, 2014, 2:41 p.m. UTC | #7
On 5/14/14, 9:39 AM, Goffredo Baroncelli wrote:
> Hi Eric,
> 
> On 05/14/2014 03:18 AM, Eric Sandeen wrote:
>> Allow the specification of the filesystem UUID at mkfs time.
> 
> I suggest to add some warning when this options is used, because the
> behavior could be very different than the one expected.
> 
> I suspect that BTRFS tracks the filesystem by UUID and not by
> devices. When two filesystems have the same UUID at the same time, it
> may mount the wrong one.

Well, of course if you explicitly create two "universally unique identifiers"
which are not actually unique, you can shoot yourself in the foot.

"Here's enough rope to hang yourself with" is a proud tradition in
the Unix world.  ;)

> $ #
> $ # Make two *different* filesystems with the *same* UUID
> $ #
> $ UUID=e285c9bd-ea97-40b3-ad7d-8713dcfd5eea
> $ sudo ./mkfs.btrfs -f -U $UUID /dev/vdg
> $ sudo ./mkfs.btrfs -f -U $UUID /dev/vdh
> 
> $ #
> $ # from the beginning "btrfs fi show" reports wrong information
> $ #
> $ sudo btrfs fi show
> Label: none  uuid: e285c9bd-ea97-40b3-ad7d-8713dcfd5eea
> 	Total devices 1 FS bytes used 96.00KB
> 	devid    1 size 50.00GB used 4.00MB path /dev/vdh
> 	devid    1 size 50.00GB used 2.04GB path /dev/vdg
> 
> $ #
> $ # mount the first one, create a new file then un-mount it
> $ #
> $ sudo mount /dev/vdg /mnt/btrfs1
> $ sudo touch /mnt/btrfs1/dev-vdg
> $ sudo umount /dev/vdg
> 
> $ #
> $ # mount the second one, it should be empty
> $ # instead btrfs mount the first one
> $ #
> $ sudo mount /dev/vdh /mnt/btrfs2
> $ ls -l /mnt/btrfs2
> total 0
> -rw-r--r-- 1 root root 0 May 14 16:12 dev-vdg

o_O ok, that's a little unexpected.

> 
> I am not against this option; I am suggesting to add a explicit
> warning to the user about the risk of doing that, both on the man
> pages and into the program. The warning should say that this option
> is only for testing. Better ask for a confirmation (even with an
> undocumented switch like
> '--I-know-that-I-am-doing-something-really-dangerous').

meh.  ext4 and xfs have had the ability to either set or change the
UUID for years, and I've not heard of any horror stories.

> 
> For the record, BTRFS seems unable to mount at the same time two different filesystems with the same UUID:
> 
> $ #
> $ # try to mount two fs with the same UUID, but BTRFS doesn't allow it
> $ #
> $ sudo mount /dev/vdh /mnt/btrfs2
> $ sudo mount /dev/vdg /mnt/btrfs1
> ERROR: mount failed : 16 - Device or resource busy

(presumably the kernel said something, too?)

and this seems to make it even more safe.

-Eric

> 
> BR
> G.Baroncelli
> 

--
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
James Shubin May 14, 2014, 2:42 p.m. UTC | #8
On Wed, 2014-05-14 at 14:25 +0200, Brendan Hide wrote:
> On 14/05/14 09:31, Wang Shilong wrote:
> > On 05/14/2014 09:18 AM, Eric Sandeen wrote:
> >> Allow the specification of the filesystem UUID at mkfs time.
> >>
> >> (Implemented only for mkfs.btrfs, not btrfs-convert).
> > Just out of curiosity, this option is used for what kind of use case?
> > I notice Ext4 also has this option.:-)
> Personally I can't think of any "average" or "normal" use case. The 
> simplest case however is in using predictable/predetermined UUIDs.
> 
> Certain things, such as testing or perhaps even large-scale automation, 
> are likely simpler to implement with a predictable UUID.
> 
Exactly this! My use case is actually for storage automation. By using
an internally generated UUID, it's one easy way to "track" which
filesystem your automation code made.

Example:

https://github.com/purpleidea/puppet-gluster/blob/master/manifests/brick.pp#L469

HTH,
James
James Shubin May 14, 2014, 2:47 p.m. UTC | #9
On Wed, 2014-05-14 at 16:39 +0200, Goffredo Baroncelli wrote:
> Hi Eric,
> 
> On 05/14/2014 03:18 AM, Eric Sandeen wrote:
> > Allow the specification of the filesystem UUID at mkfs time.
> 
> I suggest to add some warning when this options is used, because the behavior could be very different than the one expected.
> 
> I suspect that BTRFS tracks the filesystem by UUID and not by devices. When two filesystems have the same UUID at the same time, it may mount the wrong one.
> 
> $ #
> $ # Make two *different* filesystems with the *same* UUID
> $ #
> $ UUID=e285c9bd-ea97-40b3-ad7d-8713dcfd5eea
> $ sudo ./mkfs.btrfs -f -U $UUID /dev/vdg
> $ sudo ./mkfs.btrfs -f -U $UUID /dev/vdh
> 
> $ #
> $ # from the beginning "btrfs fi show" reports wrong information
> $ #
> $ sudo btrfs fi show
> Label: none  uuid: e285c9bd-ea97-40b3-ad7d-8713dcfd5eea
> 	Total devices 1 FS bytes used 96.00KB
> 	devid    1 size 50.00GB used 4.00MB path /dev/vdh
> 	devid    1 size 50.00GB used 2.04GB path /dev/vdg
> 
> $ #
> $ # mount the first one, create a new file then un-mount it
> $ #
> $ sudo mount /dev/vdg /mnt/btrfs1
> $ sudo touch /mnt/btrfs1/dev-vdg
> $ sudo umount /dev/vdg
> 
> $ #
> $ # mount the second one, it should be empty
> $ # instead btrfs mount the first one
> $ #
> $ sudo mount /dev/vdh /mnt/btrfs2
> $ ls -l /mnt/btrfs2
> total 0
> -rw-r--r-- 1 root root 0 May 14 16:12 dev-vdg
> 
> 
> I am not against this option; I am suggesting to add a explicit warning to the user about the risk of doing that, both on the man pages and into the program. 
> The warning should say that this option is only for testing. Better ask for a confirmation (even with an undocumented switch like '--I-know-that-I-am-doing-something-really-dangerous').

I don't think is necessary since xfs and ext4 both have these options
too... There probably *should* be a conflict, because typically this
will cause an entry to show up in
/dev/disks/by-uuid/$UUID

I'm sure this same bug exists if two "randomly generated UUID's" get
created too. I think a warning in the man page would be enough.

> 
> For the record, BTRFS seems unable to mount at the same time two different filesystems with the same UUID:
> 
> $ #
> $ # try to mount two fs with the same UUID, but BTRFS doesn't allow it
> $ #
> $ sudo mount /dev/vdh /mnt/btrfs2
> $ sudo mount /dev/vdg /mnt/btrfs1
> ERROR: mount failed : 16 - Device or resource busy
> 
> 
> BR
> G.Baroncelli
>
Goffredo Baroncelli May 14, 2014, 3:14 p.m. UTC | #10
On 05/14/2014 04:41 PM, Eric Sandeen wrote:

>> I am not against this option; I am suggesting to add a explicit
>> warning to the user about the risk of doing that, both on the man
>> pages and into the program. The warning should say that this option
>> is only for testing. Better ask for a confirmation (even with an
>> undocumented switch like
>> '--I-know-that-I-am-doing-something-really-dangerous').
> 
> meh.  ext4 and xfs have had the ability to either set or change the
> UUID for years, and I've not heard of any horror stories.

BTRFS tracks the filesystem by UUID, so it behaves strange when two FS have the same UUID. This is the reason for adding further warning. If it would behave like ext4/xfs I didn't suggested to add any warning.

> 
>>
>> For the record, BTRFS seems unable to mount at the same time two different filesystems with the same UUID:
>>
>> $ #
>> $ # try to mount two fs with the same UUID, but BTRFS doesn't allow it
>> $ #
>> $ sudo mount /dev/vdh /mnt/btrfs2
>> $ sudo mount /dev/vdg /mnt/btrfs1
>> ERROR: mount failed : 16 - Device or resource busy
> 
> (presumably the kernel said something, too?)
I see nothing. Maybe it thinks that these are two disks of a multivolume-filesystem...
> 
> and this seems to make it even more safe.
> 
> -Eric
> 
>>
>> BR
>> G.Baroncelli
>>
> 
> --
> 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 May 14, 2014, 3:27 p.m. UTC | #11
On Wed, May 14, 2014 at 09:41:19AM -0500, Eric Sandeen wrote:
> > I am not against this option; I am suggesting to add a explicit
> > warning to the user about the risk of doing that, both on the man
> > pages and into the program. The warning should say that this option
> > is only for testing. Better ask for a confirmation (even with an
> > undocumented switch like
> > '--I-know-that-I-am-doing-something-really-dangerous').
> 
> meh.  ext4 and xfs have had the ability to either set or change the
> UUID for years, and I've not heard of any horror stories.

Because these filesystems do not detect their devices by uuid, it's
mainly for the multi-device fs, but applies for single-device fs as well.
--
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
diff mbox

Patch

diff --git a/btrfs-convert.c b/btrfs-convert.c
index a8b2c51..d62d4f8 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -2240,7 +2240,7 @@  static int do_convert(const char *devname, int datacsum, int packing,
 		goto fail;
 	}
 	ret = make_btrfs(fd, devname, ext2_fs->super->s_volume_name,
-			 blocks, total_bytes, blocksize, blocksize,
+			 NULL, blocks, total_bytes, blocksize, blocksize,
 			 blocksize, blocksize, 0);
 	if (ret) {
 		fprintf(stderr, "unable to create initial ctree: %s\n",
diff --git a/man/mkfs.btrfs.8.in b/man/mkfs.btrfs.8.in
index bef509d..4450d03 100644
--- a/man/mkfs.btrfs.8.in
+++ b/man/mkfs.btrfs.8.in
@@ -16,6 +16,7 @@  mkfs.btrfs \- create a btrfs filesystem
 [ \fB\-r\fP\fI rootdir\fP ]
 [ \fB\-K\fP ]
 [ \fB\-O\fP\fI feature1,feature2,...\fP ]
+[ \fB\-U\fP\fI uuid\fP ]
 [ \fB\-h\fP ]
 [ \fB\-V\fP ]
 \fI device\fP [ \fIdevice ...\fP ]
@@ -90,6 +91,9 @@  To see all run
 
 \fBmkfs.btrfs -O list-all\fR
 .TP
+\fB\-U\fR, \fB\-\-uuid \fR
+Create the filesystem with the specified UUID.
+.TP
 \fB\-V\fR, \fB\-\-version\fR
 Print the \fBmkfs.btrfs\fP version and exit.
 .SH UNIT
diff --git a/mkfs.c b/mkfs.c
index dbd83f5..eccb08f 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -288,6 +288,7 @@  static void print_usage(void)
 	fprintf(stderr, "\t -r --rootdir the source directory\n");
 	fprintf(stderr, "\t -K --nodiscard do not perform whole device TRIM\n");
 	fprintf(stderr, "\t -O --features comma separated list of filesystem features\n");
+	fprintf(stderr, "\t -U --uuid specify the filesystem UUID\n");
 	fprintf(stderr, "\t -V --version print the mkfs.btrfs version and exit\n");
 	fprintf(stderr, "%s\n", BTRFS_BUILD_VERSION);
 	exit(1);
@@ -351,6 +352,7 @@  static struct option long_options[] = {
 	{ "rootdir", 1, NULL, 'r' },
 	{ "nodiscard", 0, NULL, 'K' },
 	{ "features", 0, NULL, 'O' },
+	{ "uuid", 0, NULL, 'U' },
 	{ NULL, 0, NULL, 0}
 };
 
@@ -1273,11 +1275,12 @@  int main(int ac, char **av)
 	int dev_cnt = 0;
 	int saved_optind;
 	char estr[100];
+	char *fs_uuid = NULL;
 	u64 features = DEFAULT_MKFS_FEATURES;
 
 	while(1) {
 		int c;
-		c = getopt_long(ac, av, "A:b:fl:n:s:m:d:L:O:r:VMK",
+		c = getopt_long(ac, av, "A:b:fl:n:s:m:d:L:O:r:U:VMK",
 				long_options, &option_index);
 		if (c < 0)
 			break;
@@ -1346,6 +1349,9 @@  int main(int ac, char **av)
 				source_dir = optarg;
 				source_dir_set = 1;
 				break;
+			case 'U':
+				fs_uuid = optarg;
+				break;
 			case 'K':
 				discard = 0;
 				break;
@@ -1514,7 +1520,7 @@  int main(int ac, char **av)
 
 	process_fs_features(features);
 
-	ret = make_btrfs(fd, file, label, blocks, dev_block_count,
+	ret = make_btrfs(fd, file, label, fs_uuid, blocks, dev_block_count,
 			 nodesize, leafsize,
 			 sectorsize, stripesize, features);
 	if (ret) {
diff --git a/utils.c b/utils.c
index 3e9c527..3f46e8f 100644
--- a/utils.c
+++ b/utils.c
@@ -93,12 +93,12 @@  static u64 reference_root_table[] = {
 	[6] =	BTRFS_CSUM_TREE_OBJECTID,
 };
 
-int make_btrfs(int fd, const char *device, const char *label,
+int make_btrfs(int fd, const char *device, const char *label, char *fs_uuid,
 	       u64 blocks[7], u64 num_bytes, u32 nodesize,
 	       u32 leafsize, u32 sectorsize, u32 stripesize, u64 features)
 {
 	struct btrfs_super_block super;
-	struct extent_buffer *buf;
+	struct extent_buffer *buf = NULL;
 	struct btrfs_root_item root_item;
 	struct btrfs_disk_key disk_key;
 	struct btrfs_extent_item *extent_item;
@@ -125,7 +125,14 @@  int make_btrfs(int fd, const char *device, const char *label,
 	memset(&super, 0, sizeof(super));
 
 	num_bytes = (num_bytes / sectorsize) * sectorsize;
-	uuid_generate(super.fsid);
+	if (fs_uuid) {
+		if (uuid_parse(fs_uuid, super.fsid) != 0) {
+			fprintf(stderr, "could not parse UUID: %s\n", fs_uuid);
+			ret = -EINVAL;
+			goto out;
+		}
+	} else
+		uuid_generate(super.fsid);
 	uuid_generate(super.dev_item.uuid);
 	uuid_generate(chunk_tree_uuid);
 
diff --git a/utils.h b/utils.h
index 3c62066..054cbb6 100644
--- a/utils.h
+++ b/utils.h
@@ -40,7 +40,7 @@ 
 #define BTRFS_UUID_UNPARSED_SIZE	37
 
 int make_btrfs(int fd, const char *device, const char *label,
-	       u64 blocks[6], u64 num_bytes, u32 nodesize,
+	       char *fs_uuid, u64 blocks[6], u64 num_bytes, u32 nodesize,
 	       u32 leafsize, u32 sectorsize, u32 stripesize, u64 features);
 int btrfs_make_root_dir(struct btrfs_trans_handle *trans,
 			struct btrfs_root *root, u64 objectid);