[U-Boot] [PATCH] gpt: support random UUIDs without setting environment variables
Przemyslaw Marczak
p.marczak at samsung.com
Mon Feb 23 15:42:13 CET 2015
Hello,
On 02/22/2015 11:46 PM, Rob Herring wrote:
> On Wed, Feb 18, 2015 at 11:48 AM, Przemyslaw Marczak
> <p.marczak at 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 at 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 at samsung.com>
Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
More information about the U-Boot
mailing list