* [RFC PATCH] Btrfs-progs: Add optional 'uuid' parameter to mkfs.btrfs
@ 2014-01-08 21:50 Timothy Pepper
2014-01-14 16:06 ` David Sterba
0 siblings, 1 reply; 4+ messages in thread
From: Timothy Pepper @ 2014-01-08 21:50 UTC (permalink / raw
To: linux-btrfs
The patch below is a simple quick attempt at allowing the filesystem
UUID to be specified by the user at mkfs time. Googling around I've
seen a lot of people wishing they could "change their btrfs uuid".
I understand why that's not going to happen. But this little patch
seems like it could give them a control somewhat equivalent to what
they're seeking. It works for me anyway, so I thought I'd share it in
case it is upstream acceptable and useful to others.
--
Tim Pepper <timothy.c.pepper@linux.intel.com>
Intel Open Source Technology Center
==============================================================================
From: Tim Pepper <timothy.c.pepper@linux.intel.com>
Date: Wed, 8 Jan 2014 11:20:26 -0800
Subject: [PATCH] Add optional 'uuid' parameter to mkfs.btrfs
Currently the code supports specifying a label, but not UUID. Since the
UUID of a btrfs filesystem can't be modified after the filesystem is made,
it is useful to support specifying the UUID when the filesystem is made.
Signed-off-by: Tim Pepper <timothy.c.pepper@linux.intel.com>
---
btrfs-convert.c | 2 +-
mkfs.c | 23 +++++++++++++++++++++--
utils.c | 7 +++++--
utils.h | 2 +-
4 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/btrfs-convert.c b/btrfs-convert.c
index ae10eed..7ec1ac5 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -2242,7 +2242,7 @@ static int do_convert(const char *devname, int datacsum, int packing,
fprintf(stderr, "unable to open %s\n", devname);
goto fail;
}
- ret = make_btrfs(fd, devname, ext2_fs->super->s_volume_name,
+ ret = make_btrfs(fd, devname, ext2_fs->super->s_volume_name, NULL,
blocks, total_bytes, blocksize, blocksize,
blocksize, blocksize, 0);
if (ret) {
diff --git a/mkfs.c b/mkfs.c
index 33369f9..d5e65d9 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -280,6 +280,7 @@ static void print_usage(void)
fprintf(stderr, "\t -f --force force overwrite of existing filesystem\n");
fprintf(stderr, "\t -l --leafsize size of btree leaves\n");
fprintf(stderr, "\t -L --label set a label\n");
+ fprintf(stderr, "\t -U --uuid set a UUID\n");
fprintf(stderr, "\t -m --metadata metadata profile, values like data profile\n");
fprintf(stderr, "\t -M --mixed mix metadata and data together\n");
fprintf(stderr, "\t -n --nodesize size of btree nodes\n");
@@ -335,12 +336,25 @@ static char *parse_label(char *input)
return strdup(input);
}
+static char *parse_uuid(char *input)
+{
+ int len = strlen(input);
+#define BTRFS_UUID_STRING_SIZE (2*BTRFS_UUID_SIZE + 5)
+ if (len >= BTRFS_UUID_STRING_SIZE) {
+ fprintf(stderr, "UUID %s is too long (max %d)\n", input,
+ BTRFS_UUID_STRING_SIZE - 1);
+ exit(1);
+ }
+ return strdup(input);
+}
+
static struct option long_options[] = {
{ "alloc-start", 1, NULL, 'A'},
{ "byte-count", 1, NULL, 'b' },
{ "force", 0, NULL, 'f' },
{ "leafsize", 1, NULL, 'l' },
{ "label", 1, NULL, 'L'},
+ { "uuid", 1, NULL, 'U'},
{ "metadata", 1, NULL, 'm' },
{ "mixed", 0, NULL, 'M' },
{ "nodesize", 1, NULL, 'n' },
@@ -1228,6 +1242,7 @@ int main(int ac, char **av)
struct btrfs_root *root;
struct btrfs_trans_handle *trans;
char *label = NULL;
+ char *uuid = NULL;
char *first_file;
u64 block_count = 0;
u64 dev_block_count = 0;
@@ -1264,7 +1279,7 @@ int main(int ac, char **av)
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:U:O:r:VMK",
long_options, &option_index);
if (c < 0)
break;
@@ -1288,6 +1303,9 @@ int main(int ac, char **av)
case 'L':
label = parse_label(optarg);
break;
+ case 'U':
+ uuid = parse_uuid(optarg);
+ break;
case 'm':
metadata_profile = parse_profile(optarg);
metadata_profile_opt = 1;
@@ -1497,7 +1515,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, uuid, blocks, dev_block_count,
nodesize, leafsize,
sectorsize, stripesize, features);
if (ret) {
@@ -1595,5 +1613,6 @@ raid_groups:
ret = close_ctree(root);
BUG_ON(ret);
free(label);
+ free(uuid);
return 0;
}
diff --git a/utils.c b/utils.c
index f499023..4b85eeb 100644
--- a/utils.c
+++ b/utils.c
@@ -71,7 +71,7 @@ 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, const char *uuid,
u64 blocks[7], u64 num_bytes, u32 nodesize,
u32 leafsize, u32 sectorsize, u32 stripesize, u64 features)
{
@@ -103,7 +103,10 @@ 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 (uuid)
+ uuid_parse(uuid, super.fsid);
+ 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 6f4b10c..3b89ece 100644
--- a/utils.h
+++ b/utils.h
@@ -37,7 +37,7 @@
#define BTRFS_ARG_UUID 2
#define BTRFS_ARG_BLKDEV 3
-int make_btrfs(int fd, const char *device, const char *label,
+int make_btrfs(int fd, const char *device, const char *label, const char *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,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] Btrfs-progs: Add optional 'uuid' parameter to mkfs.btrfs
2014-01-08 21:50 [RFC PATCH] Btrfs-progs: Add optional 'uuid' parameter to mkfs.btrfs Timothy Pepper
@ 2014-01-14 16:06 ` David Sterba
2014-01-15 0:40 ` Timothy Pepper
0 siblings, 1 reply; 4+ messages in thread
From: David Sterba @ 2014-01-14 16:06 UTC (permalink / raw
To: Timothy Pepper; +Cc: linux-btrfs
On Wed, Jan 08, 2014 at 01:50:58PM -0800, Timothy Pepper wrote:
> The patch below is a simple quick attempt at allowing the filesystem
> UUID to be specified by the user at mkfs time.
There was a similar patch some time ago, that also added it to convert:
http://www.spinics.net/lists/linux-btrfs/msg21193.html
and my comments apply to your patch as well:
http://www.spinics.net/lists/linux-btrfs/msg22889.html
"Can you please enhance it with a check that the UUID is not duplicate
among other filesystems? It should be available through libblkid:
blkid_probe_lookup_value(probe, "UUID", &uuid, NULL);
This is a sort of paranoia check with the generated UUID, but I think
that an accidentally repeated command or copy&paste with an existing
uuid can happen."
> Googling around I've
> seen a lot of people wishing they could "change their btrfs uuid".
> I understand why that's not going to happen.
Well, the user demand can make it happen. The offline rewrite is easy
but has it's own drawbacks.
It seems doable to do a mount-time change of uuid. Mount a fs under a
different uuid that would become the new one, while still accepting the
previuos uuid when found in metadata. A full scrub pass would convert
the old uuids when reqested.
(This will become a wiki://Project_idea eventually)
> @@ -335,12 +336,25 @@ static char *parse_label(char *input)
> return strdup(input);
> }
>
> +static char *parse_uuid(char *input)
please use libuuid API instead
> +{
> + int len = strlen(input);
> +#define BTRFS_UUID_STRING_SIZE (2*BTRFS_UUID_SIZE + 5)
> + if (len >= BTRFS_UUID_STRING_SIZE) {
> + fprintf(stderr, "UUID %s is too long (max %d)\n", input,
> + BTRFS_UUID_STRING_SIZE - 1);
> + exit(1);
> + }
> + return strdup(input);
> +}
> +
> @@ -1288,6 +1303,9 @@ int main(int ac, char **av)
> case 'L':
> label = parse_label(optarg);
> break;
> + case 'U':
> + uuid = parse_uuid(optarg);
Error handling needed.
> + break;
> case 'm':
> metadata_profile = parse_profile(optarg);
> metadata_profile_opt = 1;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] Btrfs-progs: Add optional 'uuid' parameter to mkfs.btrfs
2014-01-14 16:06 ` David Sterba
@ 2014-01-15 0:40 ` Timothy Pepper
2014-01-15 1:16 ` Qu Wenruo
0 siblings, 1 reply; 4+ messages in thread
From: Timothy Pepper @ 2014-01-15 0:40 UTC (permalink / raw
To: dsterba, linux-btrfs
On Tue 14 Jan at 17:06:29 +0100 dsterba@suse.cz said:
> On Wed, Jan 08, 2014 at 01:50:58PM -0800, Timothy Pepper wrote:
> > The patch below is a simple quick attempt at allowing the filesystem
> > UUID to be specified by the user at mkfs time.
>
> There was a similar patch some time ago, that also added it to convert:
> http://www.spinics.net/lists/linux-btrfs/msg21193.html
>
> and my comments apply to your patch as well:
> http://www.spinics.net/lists/linux-btrfs/msg22889.html
>
> "Can you please enhance it with a check that the UUID is not duplicate
> among other filesystems? It should be available through libblkid:
>
> blkid_probe_lookup_value(probe, "UUID", &uuid, NULL);
>
> This is a sort of paranoia check with the generated UUID, but I think
> that an accidentally repeated command or copy&paste with an existing
> uuid can happen."
This makes sense, but feels like a separate and distinct need. I've
simply followed the existing implementation for user specified labels.
Both arguably should have additional logic for interacting with the user
or making explicit whether a duplicate UUID/LABEL is or is not allowable
to the user. In my oddball use case I happen to be ok with a duplicate
UUID's and explicitly want them ;)
>
> > @@ -335,12 +336,25 @@ static char *parse_label(char *input)
> > return strdup(input);
> > }
> >
> > +static char *parse_uuid(char *input)
>
> please use libuuid API instead
As in uuid_parse(), which I use later?
> > +{
> > + int len = strlen(input);
> > +#define BTRFS_UUID_STRING_SIZE (2*BTRFS_UUID_SIZE + 5)
> > + if (len >= BTRFS_UUID_STRING_SIZE) {
> > + fprintf(stderr, "UUID %s is too long (max %d)\n", input,
> > + BTRFS_UUID_STRING_SIZE - 1);
> > + exit(1);
> > + }
> > + return strdup(input);
> > +}
> > +
> > @@ -1288,6 +1303,9 @@ int main(int ac, char **av)
> > case 'L':
> > label = parse_label(optarg);
> > break;
> > + case 'U':
> > + uuid = parse_uuid(optarg);
>
> Error handling needed.
The uuid is initialized to NULL, same as label. The two
parse_{uuid,name}() functions return a strdup of the user input or
exit(1). If strdup returned NULL, it's the same is if the user gave no
input and the code didn't hit the 'L' and/or 'U' cases.
The patch may make more sense in the context of the full file where you
see it mirroring the user specified label case in the lines just prior
to each addition hunk in mkfs.c.
This makes for a simpler, more natural addition in utils.c where I've
done the following hunk:
diff --git a/utils.c b/utils.c
index f499023..4b85eeb 100644
--- a/utils.c
+++ b/utils.c
@@ -71,7 +71,7 @@ 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, const char *uuid,
u64 blocks[7], u64 num_bytes, u32 nodesize,
u32 leafsize, u32 sectorsize, u32 stripesize, u64 features)
{
@@ -103,7 +103,10 @@ 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 (uuid)
+ uuid_parse(uuid, super.fsid);
+ else
+ uuid_generate(super.fsid);
uuid_generate(super.dev_item.uuid);
uuid_generate(chunk_tree_uuid);
I could remove the strdup in mkfs.c, move the uuid_parse() up into mkfs.c,
and do a memcpy into super down here. The way I did it seem the easiest
to understand for overall code readability within each of mkfs.c and
utils.c, keeping a similar set of initializations in mkfs.c and the set
of uuid_*() calls next to each other in utils.c.
After that hunk in utils.c, and even without my change, the code
could/should check whether these three uuid's are unique and enable policy
choice there.
--
Tim Pepper <timothy.c.pepper@linux.intel.com>
Intel Open Source Technology Center
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] Btrfs-progs: Add optional 'uuid' parameter to mkfs.btrfs
2014-01-15 0:40 ` Timothy Pepper
@ 2014-01-15 1:16 ` Qu Wenruo
0 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2014-01-15 1:16 UTC (permalink / raw
To: Timothy Pepper, dsterba, linux-btrfs
On tue, 14 Jan 2014 16:40:28 -0800, Timothy Pepper wrote:
> On Tue 14 Jan at 17:06:29 +0100 dsterba@suse.cz said:
>> On Wed, Jan 08, 2014 at 01:50:58PM -0800, Timothy Pepper wrote:
>>> The patch below is a simple quick attempt at allowing the filesystem
>>> UUID to be specified by the user at mkfs time.
>> There was a similar patch some time ago, that also added it to convert:
>> http://www.spinics.net/lists/linux-btrfs/msg21193.html
>>
>> and my comments apply to your patch as well:
>> http://www.spinics.net/lists/linux-btrfs/msg22889.html
>>
>> "Can you please enhance it with a check that the UUID is not duplicate
>> among other filesystems? It should be available through libblkid:
>>
>> blkid_probe_lookup_value(probe, "UUID", &uuid, NULL);
>>
>> This is a sort of paranoia check with the generated UUID, but I think
>> that an accidentally repeated command or copy&paste with an existing
>> uuid can happen."
If my memory serves me correctly, chunk recovery in btrfsck does
a full disk scan to collect the chunk/block group info,
and use fs uuid to distinguish previous btrfs data and current btrfs data.
If a user always mkfs.btrfs with the same UUID,
which I think is highly possible just for not touching the fstab,
when they want to recover from a chunk/block group problem by using
btrfsck, a disaster may happen.
So I prefer the LABEL way if users really mkfs frequently.
Thanks
Qu
> This makes sense, but feels like a separate and distinct need. I've
> simply followed the existing implementation for user specified labels.
> Both arguably should have additional logic for interacting with the user
> or making explicit whether a duplicate UUID/LABEL is or is not allowable
> to the user. In my oddball use case I happen to be ok with a duplicate
> UUID's and explicitly want them ;)
>
>>> @@ -335,12 +336,25 @@ static char *parse_label(char *input)
>>> return strdup(input);
>>> }
>>>
>>> +static char *parse_uuid(char *input)
>> please use libuuid API instead
> As in uuid_parse(), which I use later?
>
>>> +{
>>> + int len = strlen(input);
>>> +#define BTRFS_UUID_STRING_SIZE (2*BTRFS_UUID_SIZE + 5)
>>> + if (len >= BTRFS_UUID_STRING_SIZE) {
>>> + fprintf(stderr, "UUID %s is too long (max %d)\n", input,
>>> + BTRFS_UUID_STRING_SIZE - 1);
>>> + exit(1);
>>> + }
>>> + return strdup(input);
>>> +}
>>> +
>>> @@ -1288,6 +1303,9 @@ int main(int ac, char **av)
>>> case 'L':
>>> label = parse_label(optarg);
>>> break;
>>> + case 'U':
>>> + uuid = parse_uuid(optarg);
>> Error handling needed.
> The uuid is initialized to NULL, same as label. The two
> parse_{uuid,name}() functions return a strdup of the user input or
> exit(1). If strdup returned NULL, it's the same is if the user gave no
> input and the code didn't hit the 'L' and/or 'U' cases.
>
> The patch may make more sense in the context of the full file where you
> see it mirroring the user specified label case in the lines just prior
> to each addition hunk in mkfs.c.
>
> This makes for a simpler, more natural addition in utils.c where I've
> done the following hunk:
>
> diff --git a/utils.c b/utils.c
> index f499023..4b85eeb 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -71,7 +71,7 @@ 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, const char *uuid,
> u64 blocks[7], u64 num_bytes, u32 nodesize,
> u32 leafsize, u32 sectorsize, u32 stripesize, u64 features)
> {
> @@ -103,7 +103,10 @@ 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 (uuid)
> + uuid_parse(uuid, super.fsid);
> + else
> + uuid_generate(super.fsid);
> uuid_generate(super.dev_item.uuid);
> uuid_generate(chunk_tree_uuid);
>
> I could remove the strdup in mkfs.c, move the uuid_parse() up into mkfs.c,
> and do a memcpy into super down here. The way I did it seem the easiest
> to understand for overall code readability within each of mkfs.c and
> utils.c, keeping a similar set of initializations in mkfs.c and the set
> of uuid_*() calls next to each other in utils.c.
>
> After that hunk in utils.c, and even without my change, the code
> could/should check whether these three uuid's are unique and enable policy
> choice there.
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-01-15 1:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-08 21:50 [RFC PATCH] Btrfs-progs: Add optional 'uuid' parameter to mkfs.btrfs Timothy Pepper
2014-01-14 16:06 ` David Sterba
2014-01-15 0:40 ` Timothy Pepper
2014-01-15 1:16 ` Qu Wenruo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.