* [U-Boot] [PATCH] gpt: support random UUIDs without setting environment variables @ 2015-01-26 15:44 Rob Herring 2015-02-18 17:48 ` Przemyslaw Marczak 2015-03-06 15:43 ` [U-Boot] " Tom Rini 0 siblings, 2 replies; 5+ messages in thread From: Rob Herring @ 2015-01-26 15:44 UTC (permalink / raw) To: u-boot Currently, an environment variable must be used to store the randomly generated UUID for each partition. This is not necessary, so make storing the UUID optional. Now passing uuid_disk and uuid are optional when random UUIDs are enabled. Signed-off-by: Rob Herring <robh@kernel.org> --- common/cmd_gpt.c | 48 ++++++++++++++++++++++++++++++------------------ doc/README.gpt | 8 +++++--- 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/common/cmd_gpt.c b/common/cmd_gpt.c index 75df3fe..c56fe15 100644 --- a/common/cmd_gpt.c +++ b/common/cmd_gpt.c @@ -154,17 +154,24 @@ static int set_gpt_info(block_dev_desc_t *dev_desc, /* extract disk guid */ s = str; - tok = strsep(&s, ";"); - val = extract_val(tok, "uuid_disk"); + val = extract_val(str, "uuid_disk"); if (!val) { +#ifdef CONFIG_RANDOM_UUID + *str_disk_guid = malloc(UUID_STR_LEN + 1); + gen_rand_uuid_str(*str_disk_guid, UUID_STR_FORMAT_STD); +#else free(str); return -2; +#endif + } else { + val = strsep(&val, ";"); + if (extract_env(val, &p)) + p = val; + *str_disk_guid = strdup(p); + free(val); + /* Move s to first partition */ + strsep(&s, ";"); } - if (extract_env(val, &p)) - p = val; - *str_disk_guid = strdup(p); - free(val); - if (strlen(s) == 0) return -3; @@ -192,20 +199,25 @@ static int set_gpt_info(block_dev_desc_t *dev_desc, /* uuid */ val = extract_val(tok, "uuid"); - if (!val) { /* 'uuid' is mandatory */ - errno = -4; - goto err; - } - if (extract_env(val, &p)) - p = val; - if (strlen(p) >= sizeof(parts[i].uuid)) { - printf("Wrong uuid format for partition %d\n", i); + if (!val) { + /* 'uuid' is optional if random uuid's are enabled */ +#ifdef CONFIG_RANDOM_UUID + gen_rand_uuid_str(parts[i].uuid, UUID_STR_FORMAT_STD); +#else errno = -4; goto err; +#endif + } else { + if (extract_env(val, &p)) + p = val; + if (strlen(p) >= sizeof(parts[i].uuid)) { + printf("Wrong uuid format for partition %d\n", i); + errno = -4; + goto err; + } + strcpy((char *)parts[i].uuid, p); + free(val); } - strcpy((char *)parts[i].uuid, p); - free(val); - /* name */ val = extract_val(tok, "name"); if (!val) { /* name is mandatory */ diff --git a/doc/README.gpt b/doc/README.gpt index ec0156d..59fdeeb 100644 --- a/doc/README.gpt +++ b/doc/README.gpt @@ -157,11 +157,13 @@ To restore GUID partition table one needs to: "partitions=uuid_disk=${uuid_gpt_disk};name=${uboot_name}, size=${uboot_size},uuid=${uboot_uuid};" - Fields 'name', 'size' and 'uuid' are mandatory for every partition. + The fields 'name' and 'size' are mandatory for every partition. The field 'start' is optional. - option: CONFIG_RANDOM_UUID - If any partition "UUID" no exists then it is randomly generated. + The fields 'uuid' and 'uuid_disk' are optional if CONFIG_RANDOM_UUID is + enabled. A random uuid will be used if omitted or they point to an empty/ + non-existent environment variable. The environment variable will be set to + the generated UUID. 2. Define 'CONFIG_EFI_PARTITION' and 'CONFIG_CMD_GPT' -- 2.1.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] gpt: support random UUIDs without setting environment variables 2015-01-26 15:44 [U-Boot] [PATCH] gpt: support random UUIDs without setting environment variables Rob Herring @ 2015-02-18 17:48 ` Przemyslaw Marczak 2015-02-22 22:46 ` Rob Herring 2015-03-06 15:43 ` [U-Boot] " Tom Rini 1 sibling, 1 reply; 5+ messages in thread From: Przemyslaw Marczak @ 2015-02-18 17:48 UTC (permalink / raw) To: u-boot Hello Rob, Sorry for delay. On 01/26/2015 04:44 PM, Rob Herring wrote: > Currently, an environment variable must be used to store the randomly > generated UUID for each partition. This is not necessary, so make storing > the UUID optional. Now passing uuid_disk and uuid are optional when random > UUIDs are enabled. > > Signed-off-by: Rob Herring <robh@kernel.org> > --- > common/cmd_gpt.c | 48 ++++++++++++++++++++++++++++++------------------ > doc/README.gpt | 8 +++++--- > 2 files changed, 35 insertions(+), 21 deletions(-) > > diff --git a/common/cmd_gpt.c b/common/cmd_gpt.c > index 75df3fe..c56fe15 100644 > --- a/common/cmd_gpt.c > +++ b/common/cmd_gpt.c > @@ -154,17 +154,24 @@ static int set_gpt_info(block_dev_desc_t *dev_desc, > > /* extract disk guid */ > s = str; > - tok = strsep(&s, ";"); > - val = extract_val(tok, "uuid_disk"); > + val = extract_val(str, "uuid_disk"); > if (!val) { The code below is not required, since the same thing is done inside the extract_env() function. > +#ifdef CONFIG_RANDOM_UUID > + *str_disk_guid = malloc(UUID_STR_LEN + 1); > + gen_rand_uuid_str(*str_disk_guid, UUID_STR_FORMAT_STD); > +#else > free(str); > return -2; > +#endif > + } else { > + val = strsep(&val, ";"); > + if (extract_env(val, &p)) > + p = val; > + *str_disk_guid = strdup(p); > + free(val); > + /* Move s to first partition */ > + strsep(&s, ";"); > } > - if (extract_env(val, &p)) > - p = val; > - *str_disk_guid = strdup(p); > - free(val); > - > if (strlen(s) == 0) > return -3; > > @@ -192,20 +199,25 @@ static int set_gpt_info(block_dev_desc_t *dev_desc, > > /* uuid */ > val = extract_val(tok, "uuid"); > - if (!val) { /* 'uuid' is mandatory */ > - errno = -4; > - goto err; > - } > - if (extract_env(val, &p)) > - p = val; > - if (strlen(p) >= sizeof(parts[i].uuid)) { > - printf("Wrong uuid format for partition %d\n", i); > + if (!val) { The same in this place - code duplication. > + /* 'uuid' is optional if random uuid's are enabled */ > +#ifdef CONFIG_RANDOM_UUID > + gen_rand_uuid_str(parts[i].uuid, UUID_STR_FORMAT_STD); > +#else > errno = -4; > goto err; > +#endif > + } else { > + if (extract_env(val, &p)) > + p = val; > + if (strlen(p) >= sizeof(parts[i].uuid)) { > + printf("Wrong uuid format for partition %d\n", i); > + errno = -4; > + goto err; > + } > + strcpy((char *)parts[i].uuid, p); > + free(val); > } > - strcpy((char *)parts[i].uuid, p); > - free(val); > - > /* name */ > val = extract_val(tok, "name"); > if (!val) { /* name is mandatory */ > diff --git a/doc/README.gpt b/doc/README.gpt > index ec0156d..59fdeeb 100644 > --- a/doc/README.gpt > +++ b/doc/README.gpt > @@ -157,11 +157,13 @@ To restore GUID partition table one needs to: > "partitions=uuid_disk=${uuid_gpt_disk};name=${uboot_name}, > size=${uboot_size},uuid=${uboot_uuid};" > > - Fields 'name', 'size' and 'uuid' are mandatory for every partition. > + The fields 'name' and 'size' are mandatory for every partition. > The field 'start' is optional. > > - option: CONFIG_RANDOM_UUID > - If any partition "UUID" no exists then it is randomly generated. > + The fields 'uuid' and 'uuid_disk' are optional if CONFIG_RANDOM_UUID is > + enabled. A random uuid will be used if omitted or they point to an empty/ > + non-existent environment variable. The environment variable will be set to > + the generated UUID. The things from the above comment are implemented at present in mainline. > > 2. Define 'CONFIG_EFI_PARTITION' and 'CONFIG_CMD_GPT' > > If you want drop saving the uuid to env, then you can do it by modify the extract_env() function (diff): +#ifdef CONFIG_RANDOM_UUID_SKIP_SETENV + e = strdup(uuid_str); +#else setenv(s, uuid_str); - e = getenv(s); +#endif I would prefer introduce the new config like *SKIP_SETENV*, rather than drop this feature at all, since somebody could use it. Best regards, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com ^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] gpt: support random UUIDs without setting environment variables 2015-02-18 17:48 ` Przemyslaw Marczak @ 2015-02-22 22:46 ` Rob Herring 2015-02-23 14:42 ` Przemyslaw Marczak 0 siblings, 1 reply; 5+ messages in thread From: Rob Herring @ 2015-02-22 22:46 UTC (permalink / raw) To: u-boot On Wed, Feb 18, 2015 at 11:48 AM, Przemyslaw Marczak <p.marczak@samsung.com> wrote: > Hello Rob, > > Sorry for delay. > > On 01/26/2015 04:44 PM, Rob Herring wrote: >> >> Currently, an environment variable must be used to store the randomly >> generated UUID for each partition. This is not necessary, so make storing >> the UUID optional. Now passing uuid_disk and uuid are optional when random >> UUIDs are enabled. >> >> Signed-off-by: Rob Herring <robh@kernel.org> >> --- >> common/cmd_gpt.c | 48 ++++++++++++++++++++++++++++++------------------ >> doc/README.gpt | 8 +++++--- >> 2 files changed, 35 insertions(+), 21 deletions(-) >> >> diff --git a/common/cmd_gpt.c b/common/cmd_gpt.c >> index 75df3fe..c56fe15 100644 >> --- a/common/cmd_gpt.c >> +++ b/common/cmd_gpt.c >> @@ -154,17 +154,24 @@ static int set_gpt_info(block_dev_desc_t *dev_desc, >> >> /* extract disk guid */ >> s = str; >> - tok = strsep(&s, ";"); >> - val = extract_val(tok, "uuid_disk"); >> + val = extract_val(str, "uuid_disk"); >> if (!val) { > > > The code below is not required, since the same thing is done inside the > extract_env() function. Okay. > > >> +#ifdef CONFIG_RANDOM_UUID >> + *str_disk_guid = malloc(UUID_STR_LEN + 1); >> + gen_rand_uuid_str(*str_disk_guid, UUID_STR_FORMAT_STD); >> +#else >> free(str); >> return -2; >> +#endif >> + } else { >> + val = strsep(&val, ";"); >> + if (extract_env(val, &p)) >> + p = val; >> + *str_disk_guid = strdup(p); >> + free(val); >> + /* Move s to first partition */ >> + strsep(&s, ";"); >> } >> - if (extract_env(val, &p)) >> - p = val; >> - *str_disk_guid = strdup(p); >> - free(val); >> - >> if (strlen(s) == 0) >> return -3; >> >> @@ -192,20 +199,25 @@ static int set_gpt_info(block_dev_desc_t *dev_desc, >> >> /* uuid */ >> val = extract_val(tok, "uuid"); >> - if (!val) { /* 'uuid' is mandatory */ >> - errno = -4; >> - goto err; >> - } >> - if (extract_env(val, &p)) >> - p = val; >> - if (strlen(p) >= sizeof(parts[i].uuid)) { >> - printf("Wrong uuid format for partition %d\n", i); >> + if (!val) { > > > The same in this place - code duplication. > > >> + /* 'uuid' is optional if random uuid's are enabled >> */ >> +#ifdef CONFIG_RANDOM_UUID >> + gen_rand_uuid_str(parts[i].uuid, >> UUID_STR_FORMAT_STD); >> +#else >> errno = -4; >> goto err; >> +#endif >> + } else { >> + if (extract_env(val, &p)) >> + p = val; >> + if (strlen(p) >= sizeof(parts[i].uuid)) { >> + printf("Wrong uuid format for partition >> %d\n", i); >> + errno = -4; >> + goto err; >> + } >> + strcpy((char *)parts[i].uuid, p); >> + free(val); >> } >> - strcpy((char *)parts[i].uuid, p); >> - free(val); >> - >> /* name */ >> val = extract_val(tok, "name"); >> if (!val) { /* name is mandatory */ >> diff --git a/doc/README.gpt b/doc/README.gpt >> index ec0156d..59fdeeb 100644 >> --- a/doc/README.gpt >> +++ b/doc/README.gpt >> @@ -157,11 +157,13 @@ To restore GUID partition table one needs to: >> "partitions=uuid_disk=${uuid_gpt_disk};name=${uboot_name}, >> size=${uboot_size},uuid=${uboot_uuid};" >> >> - Fields 'name', 'size' and 'uuid' are mandatory for every partition. >> + The fields 'name' and 'size' are mandatory for every partition. >> The field 'start' is optional. >> >> - option: CONFIG_RANDOM_UUID >> - If any partition "UUID" no exists then it is randomly generated. >> + The fields 'uuid' and 'uuid_disk' are optional if CONFIG_RANDOM_UUID >> is >> + enabled. A random uuid will be used if omitted or they point to an >> empty/ >> + non-existent environment variable. The environment variable will be >> set to >> + the generated UUID. > > > The things from the above comment are implemented at present in mainline. Partially yes, but as presently worded it did not make sense to me. It took me some time to figure out I had to define env variables for each uuid. So I expanded the text so the next person is not scratching their head to get this to work. > >> >> 2. Define 'CONFIG_EFI_PARTITION' and 'CONFIG_CMD_GPT' >> >> > > > If you want drop saving the uuid to env, then you can do it by modify the > extract_env() function (diff): > > +#ifdef CONFIG_RANDOM_UUID_SKIP_SETENV > + e = strdup(uuid_str); > +#else > setenv(s, uuid_str); > - > e = getenv(s); > +#endif > > > I would prefer introduce the new config like *SKIP_SETENV*, rather than drop > this feature at all, since somebody could use it. I did not drop the feature as both saving to env or not are supported and I see little reason to make it compile time, but I will look whether I can move some of this to extract_env. Rob ^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] gpt: support random UUIDs without setting environment variables 2015-02-22 22:46 ` Rob Herring @ 2015-02-23 14:42 ` Przemyslaw Marczak 0 siblings, 0 replies; 5+ messages in thread From: Przemyslaw Marczak @ 2015-02-23 14:42 UTC (permalink / raw) To: u-boot Hello, On 02/22/2015 11:46 PM, Rob Herring wrote: > On Wed, Feb 18, 2015 at 11:48 AM, Przemyslaw Marczak > <p.marczak@samsung.com> wrote: >> Hello Rob, >> >> Sorry for delay. >> >> On 01/26/2015 04:44 PM, Rob Herring wrote: >>> >>> Currently, an environment variable must be used to store the randomly >>> generated UUID for each partition. This is not necessary, so make storing >>> the UUID optional. Now passing uuid_disk and uuid are optional when random >>> UUIDs are enabled. >>> >>> Signed-off-by: Rob Herring <robh@kernel.org> >>> --- >>> common/cmd_gpt.c | 48 ++++++++++++++++++++++++++++++------------------ >>> doc/README.gpt | 8 +++++--- >>> 2 files changed, 35 insertions(+), 21 deletions(-) >>> >>> diff --git a/common/cmd_gpt.c b/common/cmd_gpt.c >>> index 75df3fe..c56fe15 100644 >>> --- a/common/cmd_gpt.c >>> +++ b/common/cmd_gpt.c >>> @@ -154,17 +154,24 @@ static int set_gpt_info(block_dev_desc_t *dev_desc, >>> >>> /* extract disk guid */ >>> s = str; >>> - tok = strsep(&s, ";"); >>> - val = extract_val(tok, "uuid_disk"); >>> + val = extract_val(str, "uuid_disk"); >>> if (!val) { >> >> >> The code below is not required, since the same thing is done inside the >> extract_env() function. > > Okay. > >> >> >>> +#ifdef CONFIG_RANDOM_UUID >>> + *str_disk_guid = malloc(UUID_STR_LEN + 1); >>> + gen_rand_uuid_str(*str_disk_guid, UUID_STR_FORMAT_STD); >>> +#else >>> free(str); >>> return -2; >>> +#endif >>> + } else { >>> + val = strsep(&val, ";"); >>> + if (extract_env(val, &p)) >>> + p = val; >>> + *str_disk_guid = strdup(p); >>> + free(val); >>> + /* Move s to first partition */ >>> + strsep(&s, ";"); >>> } >>> - if (extract_env(val, &p)) >>> - p = val; >>> - *str_disk_guid = strdup(p); >>> - free(val); >>> - >>> if (strlen(s) == 0) >>> return -3; >>> >>> @@ -192,20 +199,25 @@ static int set_gpt_info(block_dev_desc_t *dev_desc, >>> >>> /* uuid */ >>> val = extract_val(tok, "uuid"); >>> - if (!val) { /* 'uuid' is mandatory */ >>> - errno = -4; >>> - goto err; >>> - } >>> - if (extract_env(val, &p)) >>> - p = val; >>> - if (strlen(p) >= sizeof(parts[i].uuid)) { >>> - printf("Wrong uuid format for partition %d\n", i); >>> + if (!val) { >> >> >> The same in this place - code duplication. >> >> >>> + /* 'uuid' is optional if random uuid's are enabled >>> */ >>> +#ifdef CONFIG_RANDOM_UUID >>> + gen_rand_uuid_str(parts[i].uuid, >>> UUID_STR_FORMAT_STD); >>> +#else >>> errno = -4; >>> goto err; >>> +#endif >>> + } else { >>> + if (extract_env(val, &p)) >>> + p = val; >>> + if (strlen(p) >= sizeof(parts[i].uuid)) { >>> + printf("Wrong uuid format for partition >>> %d\n", i); >>> + errno = -4; >>> + goto err; >>> + } >>> + strcpy((char *)parts[i].uuid, p); >>> + free(val); >>> } >>> - strcpy((char *)parts[i].uuid, p); >>> - free(val); >>> - >>> /* name */ >>> val = extract_val(tok, "name"); >>> if (!val) { /* name is mandatory */ >>> diff --git a/doc/README.gpt b/doc/README.gpt >>> index ec0156d..59fdeeb 100644 >>> --- a/doc/README.gpt >>> +++ b/doc/README.gpt >>> @@ -157,11 +157,13 @@ To restore GUID partition table one needs to: >>> "partitions=uuid_disk=${uuid_gpt_disk};name=${uboot_name}, >>> size=${uboot_size},uuid=${uboot_uuid};" >>> >>> - Fields 'name', 'size' and 'uuid' are mandatory for every partition. >>> + The fields 'name' and 'size' are mandatory for every partition. >>> The field 'start' is optional. >>> >>> - option: CONFIG_RANDOM_UUID >>> - If any partition "UUID" no exists then it is randomly generated. >>> + The fields 'uuid' and 'uuid_disk' are optional if CONFIG_RANDOM_UUID >>> is >>> + enabled. A random uuid will be used if omitted or they point to an >>> empty/ >>> + non-existent environment variable. The environment variable will be >>> set to >>> + the generated UUID. >> >> >> The things from the above comment are implemented at present in mainline. > > Partially yes, but as presently worded it did not make sense to me. It > took me some time to figure out I had to define env variables for each > uuid. So I expanded the text so the next person is not scratching > their head to get this to work. > I see, the text which you add is clear for me. >> >>> >>> 2. Define 'CONFIG_EFI_PARTITION' and 'CONFIG_CMD_GPT' >>> >>> >> >> >> If you want drop saving the uuid to env, then you can do it by modify the >> extract_env() function (diff): >> >> +#ifdef CONFIG_RANDOM_UUID_SKIP_SETENV >> + e = strdup(uuid_str); >> +#else >> setenv(s, uuid_str); >> - >> e = getenv(s); >> +#endif >> >> >> I would prefer introduce the new config like *SKIP_SETENV*, rather than drop >> this feature at all, since somebody could use it. > > I did not drop the feature as both saving to env or not are supported > and I see little reason to make it compile time, but I will look > whether I can move some of this to extract_env. > > Rob > Ah, sorry, the topic of this patch was quite misleading. At present, the random uuid is generated and saved to the env variable - I though, that you tried to drop the setenv routine. Now I see that you want to drop the need to inserting the uuid variables into the ${partitions}. Generating the random uuid was introduced to make the things easier. The feature, which you want to introduce also looks like a benefit. In this case the patch looks good. Could you please add an example of $partitions without the uuid to the README.gpt file? Acked-by: Przemyslaw Marczak <p.marczak@samsung.com> Best regards, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com ^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] gpt: support random UUIDs without setting environment variables 2015-01-26 15:44 [U-Boot] [PATCH] gpt: support random UUIDs without setting environment variables Rob Herring 2015-02-18 17:48 ` Przemyslaw Marczak @ 2015-03-06 15:43 ` Tom Rini 1 sibling, 0 replies; 5+ messages in thread From: Tom Rini @ 2015-03-06 15:43 UTC (permalink / raw) To: u-boot On Mon, Jan 26, 2015 at 09:44:18AM -0600, Rob Herring wrote: > Currently, an environment variable must be used to store the randomly > generated UUID for each partition. This is not necessary, so make storing > the UUID optional. Now passing uuid_disk and uuid are optional when random > UUIDs are enabled. > > Signed-off-by: Rob Herring <robh@kernel.org> > Acked-by: Przemyslaw Marczak <p.marczak@samsung.com> Applied to u-boot/master, thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150306/36918597/attachment.sig> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-03-06 15:43 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-26 15:44 [U-Boot] [PATCH] gpt: support random UUIDs without setting environment variables Rob Herring 2015-02-18 17:48 ` Przemyslaw Marczak 2015-02-22 22:46 ` Rob Herring 2015-02-23 14:42 ` Przemyslaw Marczak 2015-03-06 15:43 ` [U-Boot] " Tom Rini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox