Skip to contentSkip to conversation

gpt: support random UUIDs without setting environment variables

PATCH In review Rob Herring 3 participants 5 messages Jan 26, 2015 – Mar 6, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 gpt: support random UUIDs without setting environment variables

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.

---
 common/cmd_gpt.c | 48 ++++++++++++++++++++++++++++++------------------
 doc/README.gpt   |  8 +++++---
 2 files changed, 35 insertions(+), 21 deletions(-)
common/cmd_gpt.c +30 -18
index 75df3fe..c56fe15 100644
@@ -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 */
doc/README.gpt +5 -3
index ec0156d..59fdeeb 100644
@@ -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'
 
Signature · 1 line
2.1.0
#1 lore raw

PATCH 1/1 gpt: support random UUIDs without setting environment variables

Hello Rob,

Sorry for delay.

Quoted 16 lines · Rob Herring

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.

---
  common/cmd_gpt.c | 48 ++++++++++++++++++++++++++++++------------------
  doc/README.gpt   |  8 +++++---
  2 files changed, 35 insertions(+), 21 deletions(-)
common/cmd_gpt.c +0 -0
index 75df3fe..c56fe15 100644

common/cmd_gpt.c static int set_gpt_info(block_dev_desc_t *dev_desc, · line 158

Quoted 7 lines
common/cmd_gpt.c +1 -2
@@ -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.

Quoted 27 lines
common/cmd_gpt.c +13 -5
+#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 */

common/cmd_gpt.c static int set_gpt_info(block_dev_desc_t *dev_desc, · line 202

Quoted 10 lines
common/cmd_gpt.c +1 -8
  		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.

Quoted 32 lines
common/cmd_gpt.c +15 -3
+			/* '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 */
doc/README.gpt +0 -0
index ec0156d..59fdeeb 100644
@@ -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};"
 

doc/README.gpt To restore GUID partition table one needs to: · line 166

Quoted 10 lines
doc/README.gpt +5 -3
-   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.

doc/README.gpt To restore GUID partition table one needs to:

Quoted 4 lines
doc/README.gpt +0 -0
  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,

Signature · 4 lines
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
#2 lore raw

PATCH 1/1 gpt: support random UUIDs without setting environment variables

Quoted 23 lines · Przemyslaw Marczak

Hello Rob,

Sorry for delay.

Quoted 18 lines · Rob Herring

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.

---
  common/cmd_gpt.c | 48 ++++++++++++++++++++++++++++++------------------
  doc/README.gpt   |  8 +++++---
  2 files changed, 35 insertions(+), 21 deletions(-)
common/cmd_gpt.c +0 -0
index 75df3fe..c56fe15 100644
@@ -154,17 +154,24 @@ static int set_gpt_info(block_dev_desc_t *dev_desc,
 
Quoted 10 lines

common/cmd_gpt.c static int set_gpt_info(block_dev_desc_t *dev_desc, · line 158

Quoted 6 lines
common/cmd_gpt.c +1 -2
        /* 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.

Quoted 83 lines
Quoted 29 lines
common/cmd_gpt.c +13 -5
+#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 */

common/cmd_gpt.c static int set_gpt_info(block_dev_desc_t *dev_desc, · line 202

Quoted 10 lines
common/cmd_gpt.c +1 -8
                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.

Quoted 39 lines
common/cmd_gpt.c +15 -3
+                       /* '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 */
doc/README.gpt +1 -1
index ec0156d..59fdeeb 100644
@@ -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.
 
Quoted 12 lines
Quoted 9 lines
doc/README.gpt +1 -2
-   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.

Quoted 19 lines
Quoted 5 lines

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

(quoted diff) +4 -1
+#ifdef CONFIG_RANDOM_UUID_SKIP_SETENV
+               e = strdup(uuid_str);
+#else
                setenv(s, uuid_str);
-
                e = getenv(s);
+#endif
Quoted 2 lines

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

#3 lore raw

PATCH 1/1 gpt: support random UUIDs without setting environment variables

Hello,

Quoted 134 lines · Rob Herring
Quoted 23 lines · Przemyslaw Marczak

Hello Rob,

Sorry for delay.

Quoted 18 lines · Rob Herring

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.

---
   common/cmd_gpt.c | 48 ++++++++++++++++++++++++++++++------------------
   doc/README.gpt   |  8 +++++---
   2 files changed, 35 insertions(+), 21 deletions(-)
common/cmd_gpt.c +0 -0
index 75df3fe..c56fe15 100644
@@ -154,17 +154,24 @@ static int set_gpt_info(block_dev_desc_t *dev_desc,
 
Quoted 10 lines

common/cmd_gpt.c static int set_gpt_info(block_dev_desc_t *dev_desc, · line 158

Quoted 6 lines
common/cmd_gpt.c +1 -2
         /* 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.

Quoted 83 lines
Quoted 29 lines
common/cmd_gpt.c +13 -5
+#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 */

common/cmd_gpt.c static int set_gpt_info(block_dev_desc_t *dev_desc, · line 202

Quoted 10 lines
common/cmd_gpt.c +1 -8
                 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.

Quoted 39 lines
common/cmd_gpt.c +15 -3
+                       /* '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 */
doc/README.gpt +1 -1
index ec0156d..59fdeeb 100644
@@ -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.
 
Quoted 12 lines
Quoted 9 lines
doc/README.gpt +1 -2
-   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.

Quoted 5 lines

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.

Quoted 19 lines
Quoted 19 lines
Quoted 5 lines

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

(quoted diff) +4 -1
+#ifdef CONFIG_RANDOM_UUID_SKIP_SETENV
+               e = strdup(uuid_str);
+#else
                 setenv(s, uuid_str);
-
                 e = getenv(s);
+#endif
Quoted 9 lines
Quoted 2 lines

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?

Best regards,

Signature · 4 lines
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
#4 lore raw
In reply to Rob Herring
Quoted 7 lines · Rob Herring

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.

Applied to u-boot/master, thanks!

Signature · 8 lines
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>