[PATCH] mkfs.btrfs: allow UUID specification at mkfs time

[PATCH] mkfs.btrfs: allow UUID specification at mkfs time

[Eric Sandeen]

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

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

Re: [PATCH] mkfs.btrfs: allow UUID specification at mkfs time

[Wang Shilong]

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

Re: [PATCH] mkfs.btrfs: allow UUID specification at mkfs time

[Brendan Hide]

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.

-- 
__________
Brendan Hide
http://swiftspirit.co.za/
http://www.webafrica.co.za/?AFF1E97

Re: [PATCH] mkfs.btrfs: allow UUID specification at mkfs time

[Eric Sandeen]

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

Re: [PATCH] mkfs.btrfs: allow UUID specification at mkfs time

[David Pottage]

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.

-- 
David Pottage

Re: [PATCH] mkfs.btrfs: allow UUID specification at mkfs time

[Duncan]

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.

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

Re: [PATCH] mkfs.btrfs: allow UUID specification at mkfs time

[Goffredo Baroncelli]

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

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH] mkfs.btrfs: allow UUID specification at mkfs time

[Eric Sandeen]

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
> 

Re: [PATCH] mkfs.btrfs: allow UUID specification at mkfs time

[James Shubin]

[-- Attachment #1: Type: text/plain, Size: 975 bytes --]

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


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mkfs.btrfs: allow UUID specification at mkfs time

[James Shubin]

[-- Attachment #1: Type: text/plain, Size: 2502 bytes --]

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
> 



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mkfs.btrfs: allow UUID specification at mkfs time

[Goffredo Baroncelli]

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
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH] mkfs.btrfs: allow UUID specification at mkfs time

[David Sterba]

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.

[PATCH V2] mkfs.btrfs: allow UUID specification at mkfs time

[Eric Sandeen]

Allow the specification of the filesystem UUID at mkfs time.

Non-unique unique IDs are rejected.

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

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

V2: reject non-unique unique IDs.

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..48c9bb0 100644
--- a/utils.c
+++ b/utils.c
@@ -93,12 +93,41 @@ static u64 reference_root_table[] = {
 	[6] =	BTRFS_CSUM_TREE_OBJECTID,
 };
 
-int make_btrfs(int fd, const char *device, const char *label,
+int test_uuid_unique(char *fs_uuid)
+{
+	int unique = 1;
+	blkid_dev_iterate iter = NULL;
+	blkid_dev dev = NULL;
+	blkid_cache cache = NULL;
+
+	if (blkid_get_cache(&cache, 0) < 0) {
+		printf("ERROR: lblkid cache get failed\n");
+		return 1;
+	}
+	blkid_probe_all(cache);
+	iter = blkid_dev_iterate_begin(cache);
+	blkid_dev_set_search(iter, "UUID", fs_uuid);
+
+	while (blkid_dev_next(iter, &dev) == 0) {
+		dev = blkid_verify(cache, dev);
+		if (dev) {
+			unique = 0;
+			break;
+		}
+	}
+
+	blkid_dev_iterate_end(iter);
+	blkid_put_cache(cache);
+
+	return unique;
+}
+
+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 +154,19 @@ 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;
+		}
+		if (!test_uuid_unique(fs_uuid)) {
+			fprintf(stderr, "non-unique UUID: %s\n", fs_uuid);
+			ret = -EBUSY;
+			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);

Re: [PATCH V2] mkfs.btrfs: allow UUID specification at mkfs time

[David Sterba]

Thanks for adding the uuid uniqueness check, that was my major
objection for previous patch iterations,

http://www.spinics.net/lists/linux-btrfs/msg30572.html

we can now use it for convert as well (to generate or copy the uuid).

On Wed, May 14, 2014 at 10:35:05AM -0500, Eric Sandeen wrote:
> @@ -125,7 +154,19 @@ 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;

I think the uuid validity check comes too late, IMHO it should be done
after the while/getopt block outside of make_btrfs. At this point eg.
the discard or device zeroing is already done.

I would not mind to keep the check here as well as a last sanity check,
though the number of mkfs_btrfs callers is 1 and the function is not
exported to the library.

> +		}
> +		if (!test_uuid_unique(fs_uuid)) {
> +			fprintf(stderr, "non-unique UUID: %s\n", fs_uuid);
> +			ret = -EBUSY;
> +			goto out;
> +		}
> +	} else
> +		uuid_generate(super.fsid);
>  	uuid_generate(super.dev_item.uuid);
>  	uuid_generate(chunk_tree_uuid);

Re: [PATCH V2] mkfs.btrfs: allow UUID specification at mkfs time

[Eric Sandeen]

On 5/14/14, 11:01 AM, David Sterba wrote:
> Thanks for adding the uuid uniqueness check, that was my major
> objection for previous patch iterations,
> 
> http://www.spinics.net/lists/linux-btrfs/msg30572.html

Ah, thanks, I didn't know about that history, I'm sorry.

I'm not sure if my duplicate-check is over the top, you had suggested

	blkid_probe_lookup_value()

before, maybe that's simpler.  I'm not a blkid expert... but it seems to work.

> we can now use it for convert as well (to generate or copy the uuid).

woo ;)

-Eric

> On Wed, May 14, 2014 at 10:35:05AM -0500, Eric Sandeen wrote:
>> @@ -125,7 +154,19 @@ 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;
> 
> I think the uuid validity check comes too late, IMHO it should be done
> after the while/getopt block outside of make_btrfs. At this point eg.
> the discard or device zeroing is already done.
> 
> I would not mind to keep the check here as well as a last sanity check,
> though the number of mkfs_btrfs callers is 1 and the function is not
> exported to the library.
> 
>> +		}
>> +		if (!test_uuid_unique(fs_uuid)) {
>> +			fprintf(stderr, "non-unique UUID: %s\n", fs_uuid);
>> +			ret = -EBUSY;
>> +			goto out;
>> +		}
>> +	} else
>> +		uuid_generate(super.fsid);
>>  	uuid_generate(super.dev_item.uuid);
>>  	uuid_generate(chunk_tree_uuid);

Re: [PATCH V2] mkfs.btrfs: allow UUID specification at mkfs time

[Goffredo Baroncelli]

On 05/14/2014 06:01 PM, David Sterba wrote:
> On Wed, May 14, 2014 at 10:35:05AM -0500, Eric Sandeen wrote:
>> > @@ -125,7 +154,19 @@ 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;
> I think the uuid validity check comes too late, IMHO it should be done
> after the while/getopt block outside of make_btrfs. At this point eg.
> the discard or device zeroing is already done.


Pay attention that, if you don't change the test_uuid_unique() you need to perform the check after zeroing the disk, or otherwise you are not able to mkfs the same disk with the same UUID (I suspect that this is a real use case for testing).

I am still convinced that a Warning is a better way to handle these kind of situations.

As reported before, forcing a unique UUID to be not unique is a thing to skilled person. The problem is that BTRFS in this regard behaves differently respect other file-systems, and even a skilled person could not be aware of the possible problem.
In this case is better to provide a complete information, instead of complicating the things adding further check.

BR
G.Baroncelli
-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

PATCH V3] mkfs.btrfs: allow UUID specification at mkfs time

[Eric Sandeen]

Allow the specification of the filesystem UUID at mkfs time.

Non-unique unique IDs are rejected.  This includes attempting
to re-mkfs with the same UUID; if you really want to do that,
you can mkfs with a new UUID, then re-mkfs with the one you
wanted.  ;)

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

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

NB: the prior patch didn't work well if you re-mkfs'd with
the same UUID; to be honest I didn't get to the bottom of it,
but the fact that that old UUID was already in an internal
list of present devices seems to have confused things.

V2: reject non-unique unique IDs.
V3: test for non-unique unique IDs early in mkfs.

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, which must not already exist on the system.
+.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..d67a5ba 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;
@@ -1368,6 +1374,20 @@ int main(int ac, char **av)
 			"The -r option is limited to a single device\n");
 		exit(1);
 	}
+
+	if (fs_uuid) {
+		uuid_t dummy_uuid;
+
+		if (uuid_parse(fs_uuid, dummy_uuid) != 0) {
+			fprintf(stderr, "could not parse UUID: %s\n", fs_uuid);
+			exit(1);
+		}
+		if (!test_uuid_unique(fs_uuid)) {
+			fprintf(stderr, "non-unique UUID: %s\n", fs_uuid);
+			exit(1);
+		}
+	}
+	
 	while (dev_cnt-- > 0) {
 		file = av[optind++];
 		if (is_block_device(file))
@@ -1514,7 +1534,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..cfac0d4 100644
--- a/utils.c
+++ b/utils.c
@@ -93,12 +93,41 @@ static u64 reference_root_table[] = {
 	[6] =	BTRFS_CSUM_TREE_OBJECTID,
 };
 
-int make_btrfs(int fd, const char *device, const char *label,
+int test_uuid_unique(char *fs_uuid)
+{
+	int unique = 1;
+	blkid_dev_iterate iter = NULL;
+	blkid_dev dev = NULL;
+	blkid_cache cache = NULL;
+
+	if (blkid_get_cache(&cache, 0) < 0) {
+		printf("ERROR: lblkid cache get failed\n");
+		return 1;
+	}
+	blkid_probe_all(cache);
+	iter = blkid_dev_iterate_begin(cache);
+	blkid_dev_set_search(iter, "UUID", fs_uuid);
+
+	while (blkid_dev_next(iter, &dev) == 0) {
+		dev = blkid_verify(cache, dev);
+		if (dev) {
+			unique = 0;
+			break;
+		}
+	}
+
+	blkid_dev_iterate_end(iter);
+	blkid_put_cache(cache);
+
+	return unique;
+}
+
+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 +154,20 @@ 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;
+		}
+		if (!test_uuid_unique(fs_uuid)) {
+			fprintf(stderr, "non-unique UUID: %s\n", fs_uuid);
+			ret = -EBUSY;
+			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..4a404ae 100644
--- a/utils.h
+++ b/utils.h
@@ -39,8 +39,9 @@
 
 #define BTRFS_UUID_UNPARSED_SIZE	37
 
+int test_uuid_unique(char *fs_uuid);
 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);

Re: PATCH V3] mkfs.btrfs: allow UUID specification at mkfs time

[Goffredo Baroncelli]

On 05/14/2014 07:39 PM, Eric Sandeen wrote:
> Allow the specification of the filesystem UUID at mkfs time.
> 
> Non-unique unique IDs are rejected.  This includes attempting
> to re-mkfs with the same UUID; if you really want to do that,
> you can mkfs with a new UUID, then re-mkfs with the one you
> wanted.  ;)
> 
> (Implemented only for mkfs.btrfs, not btrfs-convert).
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> NB: the prior patch didn't work well if you re-mkfs'd with
> the same UUID; to be honest I didn't get to the bottom of it,
> but the fact that that old UUID was already in an internal
> list of present devices seems to have confused things.
> 
> V2: reject non-unique unique IDs.
> V3: test for non-unique unique IDs early in mkfs.
> 
> 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, which must not already exist on the system.
> +.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..d67a5ba 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;
> @@ -1368,6 +1374,20 @@ int main(int ac, char **av)
>  			"The -r option is limited to a single device\n");
>  		exit(1);
>  	}
> +
> +	if (fs_uuid) {
> +		uuid_t dummy_uuid;
> +
> +		if (uuid_parse(fs_uuid, dummy_uuid) != 0) {
> +			fprintf(stderr, "could not parse UUID: %s\n", fs_uuid);
> +			exit(1);
> +		}
> +		if (!test_uuid_unique(fs_uuid)) {
> +			fprintf(stderr, "non-unique UUID: %s\n", fs_uuid);
> +			exit(1);
> +		}

My test showed that this detect a false positive when the user tries to mkfs two time on the same device with the same uuid.

> +	}
> +	
>  	while (dev_cnt-- > 0) {
>  		file = av[optind++];
>  		if (is_block_device(file))
> @@ -1514,7 +1534,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..cfac0d4 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -93,12 +93,41 @@ static u64 reference_root_table[] = {
>  	[6] =	BTRFS_CSUM_TREE_OBJECTID,
>  };
>  
> -int make_btrfs(int fd, const char *device, const char *label,
> +int test_uuid_unique(char *fs_uuid)
> +{
> +	int unique = 1;
> +	blkid_dev_iterate iter = NULL;
> +	blkid_dev dev = NULL;
> +	blkid_cache cache = NULL;
> +
> +	if (blkid_get_cache(&cache, 0) < 0) {
> +		printf("ERROR: lblkid cache get failed\n");
> +		return 1;
> +	}
> +	blkid_probe_all(cache);
> +	iter = blkid_dev_iterate_begin(cache);
> +	blkid_dev_set_search(iter, "UUID", fs_uuid);
> +
> +	while (blkid_dev_next(iter, &dev) == 0) {

This function should skip the check for the devices involved in the mkfs.

> +		dev = blkid_verify(cache, dev);
> +		if (dev) {
> +			unique = 0;
> +			break;
> +		}
> +	}
> +
> +	blkid_dev_iterate_end(iter);
> +	blkid_put_cache(cache);
> +
> +	return unique;
> +}
> +
> +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 +154,20 @@ 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;
> +		}
> +		if (!test_uuid_unique(fs_uuid)) {
> +			fprintf(stderr, "non-unique UUID: %s\n", fs_uuid);
> +			ret = -EBUSY;
> +			goto out;
> +		}

Why a second call to test_uuid_unique(fs_uuid) ?

> +	} 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..4a404ae 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -39,8 +39,9 @@
>  
>  #define BTRFS_UUID_UNPARSED_SIZE	37
>  
> +int test_uuid_unique(char *fs_uuid);
>  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
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: PATCH V3] mkfs.btrfs: allow UUID specification at mkfs time

[Eric Sandeen]

On 5/14/14, 5:04 PM, Goffredo Baroncelli wrote:
> On 05/14/2014 07:39 PM, Eric Sandeen wrote:
>> Allow the specification of the filesystem UUID at mkfs time.
>>
>> Non-unique unique IDs are rejected.  This includes attempting
>> to re-mkfs with the same UUID; if you really want to do that,
>> you can mkfs with a new UUID, then re-mkfs with the one you
>> wanted.  ;)
>>
>> (Implemented only for mkfs.btrfs, not btrfs-convert).
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> NB: the prior patch didn't work well if you re-mkfs'd with
>> the same UUID; to be honest I didn't get to the bottom of it,
>> but the fact that that old UUID was already in an internal
>> list of present devices seems to have confused things.
>>
>> V2: reject non-unique unique IDs.
>> V3: test for non-unique unique IDs early in mkfs.
>>

<snip>

>> @@ -1368,6 +1374,20 @@ int main(int ac, char **av)
>>  			"The -r option is limited to a single device\n");
>>  		exit(1);
>>  	}
>> +
>> +	if (fs_uuid) {
>> +		uuid_t dummy_uuid;
>> +
>> +		if (uuid_parse(fs_uuid, dummy_uuid) != 0) {
>> +			fprintf(stderr, "could not parse UUID: %s\n", fs_uuid);
>> +			exit(1);
>> +		}
>> +		if (!test_uuid_unique(fs_uuid)) {
>> +			fprintf(stderr, "non-unique UUID: %s\n", fs_uuid);
>> +			exit(1);
>> +		}
> 
> My test showed that this detect a false positive when the user tries to mkfs two time on the same device with the same uuid.

It's not a false positive; after the first mkfs, the UUID does exist.  :)  See also the commit log I wrote.

>> diff --git a/utils.c b/utils.c
>> index 3e9c527..cfac0d4 100644
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -93,12 +93,41 @@ static u64 reference_root_table[] = {
>>  	[6] =	BTRFS_CSUM_TREE_OBJECTID,
>>  };
>>  
>> -int make_btrfs(int fd, const char *device, const char *label,
>> +int test_uuid_unique(char *fs_uuid)
>> +{
>> +	int unique = 1;
>> +	blkid_dev_iterate iter = NULL;
>> +	blkid_dev dev = NULL;
>> +	blkid_cache cache = NULL;
>> +
>> +	if (blkid_get_cache(&cache, 0) < 0) {
>> +		printf("ERROR: lblkid cache get failed\n");
>> +		return 1;
>> +	}
>> +	blkid_probe_all(cache);
>> +	iter = blkid_dev_iterate_begin(cache);
>> +	blkid_dev_set_search(iter, "UUID", fs_uuid);
>> +
>> +	while (blkid_dev_next(iter, &dev) == 0) {
> 
> This function should skip the check for the devices involved in the mkfs.

Perhaps.  When I was doing something similar before, I ended up with 
inexplicable segfaults when a device found on initial scan (?) got recreated
with the same UUID.  Or something.

<snip>

>> @@ -125,7 +154,20 @@ 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;
>> +		}
>> +		if (!test_uuid_unique(fs_uuid)) {
>> +			fprintf(stderr, "non-unique UUID: %s\n", fs_uuid);
>> +			ret = -EBUSY;
>> +			goto out;
>> +		}
> 
> Why a second call to test_uuid_unique(fs_uuid) ?

Because kdave said he thought it was worth being paranoid in an earlier email,
if I understood him correctly.

-Eric

Re: PATCH V3] mkfs.btrfs: allow UUID specification at mkfs time

[David Sterba]

On Wed, May 14, 2014 at 05:07:04PM -0500, Eric Sandeen wrote:
> >> @@ -125,7 +154,20 @@ 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;
> >> +		}
> >> +		if (!test_uuid_unique(fs_uuid)) {
> >> +			fprintf(stderr, "non-unique UUID: %s\n", fs_uuid);
> >> +			ret = -EBUSY;
> >> +			goto out;
> >> +		}
> > 
> > Why a second call to test_uuid_unique(fs_uuid) ?
> 
> Because kdave said he thought it was worth being paranoid in an earlier email,
> if I understood him correctly.

I'm thinking about it again. My original idea was not to easily allow
to create a duplicate uuid to a regular user. But, if one uses --uuid
already, that's something I can count as a willful action and any
mistakes can be blamed on the user.

If we end up with a warning, then the documentation should say how
spectacularly it can blow up the system.

Re: PATCH V3] mkfs.btrfs: allow UUID specification at mkfs time

[Eric Sandeen]

On 5/15/14, 12:39 PM, David Sterba wrote:
> On Wed, May 14, 2014 at 05:07:04PM -0500, Eric Sandeen wrote:
>>>> @@ -125,7 +154,20 @@ 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;
>>>> +		}
>>>> +		if (!test_uuid_unique(fs_uuid)) {
>>>> +			fprintf(stderr, "non-unique UUID: %s\n", fs_uuid);
>>>> +			ret = -EBUSY;
>>>> +			goto out;
>>>> +		}
>>>
>>> Why a second call to test_uuid_unique(fs_uuid) ?
>>
>> Because kdave said he thought it was worth being paranoid in an earlier email,
>> if I understood him correctly.
> 
> I'm thinking about it again. My original idea was not to easily allow
> to create a duplicate uuid to a regular user. But, if one uses --uuid
> already, that's something I can count as a willful action and any
> mistakes can be blamed on the user.
> 
> If we end up with a warning, then the documentation should say how
> spectacularly it can blow up the system.
> 

So, in my testing, I found that re-mkfsing a device with the same UUID lead
to weird & distant segfaults in other bits of code.  Probably due to the
uuid cache?  </handwave> - I didn't dig into it, because ...

... people didn't want to be able to create duplicate UUIDs, so I figured
the outright rejection of that was a trivial way to solve it ...

And like I mentioned, if you really want to recreate the same UUID, you
can mkfs twice.  It doesn't take long.  ;)

I dunno, maybe that's too lame.  For a feature that upstream development
doesn't really seem to want, I'm not sure how much more effort I should
put into it?

-Eric

Re: PATCH V3] mkfs.btrfs: allow UUID specification at mkfs time

[David Sterba]

On Thu, May 15, 2014 at 12:53:52PM -0500, Eric Sandeen wrote:
> So, in my testing, I found that re-mkfsing a device with the same UUID lead
> to weird & distant segfaults in other bits of code.  Probably due to the
> uuid cache?  </handwave> - I didn't dig into it, because ...

Ok, something that needs more debugging, the uuid/blkid cache is a good
guess I think.

> ... people didn't want to be able to create duplicate UUIDs, so I figured
> the outright rejection of that was a trivial way to solve it ...
> 
> And like I mentioned, if you really want to recreate the same UUID, you
> can mkfs twice.  It doesn't take long.  ;)
> 
> I dunno, maybe that's too lame.  For a feature that upstream development
> doesn't really seem to want, I'm not sure how much more effort I should
> put into it?

Well, your patch allows to create a filesytem with a given UUID in the
safe case.

There are workarounds if the UUID is not unique for the same fs (run
twice, or wipefs first).

That's enough for me to integrate the V3 patch and we can address
creating duplicate UUIDs separately once we can make it work correctly.