* [PATCH] cmd: gpt: Add option to write GPT partitions to environment variable
@ 2021-02-25 18:56 Farhan Ali
2021-02-25 20:03 ` Heinrich Schuchardt
0 siblings, 1 reply; 3+ messages in thread
From: Farhan Ali @ 2021-02-25 18:56 UTC (permalink / raw)
To: u-boot
This change would enhance the existing 'gpt read' command to allow
(optionally) writing of the read GPT partitions to an environment
variable. This would allow users to easily change the partition
settings and then simply reuse the variable in the 'gpt write' and
'gpt verify' commands.
Signed-off-by: Farhan Ali <farhan.ali@broadcom.com>
---
Cc: Simon Glass <sjg@chromium.org>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Corneliu Doban <cdoban@broadcom.com>
Cc: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
cmd/gpt.c | 46 ++++++++++++++++++++++++++++++++++++++--------
1 file changed, 38 insertions(+), 8 deletions(-)
diff --git a/cmd/gpt.c b/cmd/gpt.c
index 76a95ad..12d0551 100644
--- a/cmd/gpt.c
+++ b/cmd/gpt.c
@@ -350,17 +350,46 @@ static int get_gpt_info(struct blk_desc *dev_desc)
}
/* a wrapper to test get_gpt_info */
-static int do_get_gpt_info(struct blk_desc *dev_desc)
+static int do_get_gpt_info(struct blk_desc *dev_desc, char * const namestr)
{
- int ret;
+ int numparts;
+
+ numparts = get_gpt_info(dev_desc);
+
+ if (numparts > 0) {
+ if (namestr) {
+ char disk_guid[UUID_STR_LEN + 1];
+ char *partitions_list;
+ int partlistlen;
+ int ret = -1;
+
+ ret = get_disk_guid(dev_desc, disk_guid);
+ if (ret < 0)
+ return ret;
+
+ partlistlen = calc_parts_list_len(numparts);
+ partitions_list = malloc(partlistlen);
+ if (!partitions_list) {
+ del_gpt_info();
+ return -ENOMEM;
+ }
+ memset(partitions_list, '\0', partlistlen);
+
+ ret = create_gpt_partitions_list(numparts, disk_guid,
+ partitions_list);
+ if (ret < 0)
+ printf("Error: Could not create partition list string!\n");
+ else
+ env_set(namestr, partitions_list);
- ret = get_gpt_info(dev_desc);
- if (ret > 0) {
- print_gpt_info();
+ free(partitions_list);
+ } else {
+ print_gpt_info();
+ }
del_gpt_info();
return 0;
}
- return ret;
+ return numparts;
}
#endif
@@ -982,7 +1011,7 @@ static int do_gpt(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
ret = do_disk_guid(blk_dev_desc, argv[4]);
#ifdef CONFIG_CMD_GPT_RENAME
} else if (strcmp(argv[1], "read") == 0) {
- ret = do_get_gpt_info(blk_dev_desc);
+ ret = do_get_gpt_info(blk_dev_desc, argv[4]);
} else if ((strcmp(argv[1], "swap") == 0) ||
(strcmp(argv[1], "rename") == 0)) {
ret = do_rename_gpt_parts(blk_dev_desc, argv[1], argv[4], argv[5]);
@@ -1028,8 +1057,9 @@ U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
" gpt guid mmc 0 varname\n"
#ifdef CONFIG_CMD_GPT_RENAME
"gpt partition renaming commands:\n"
- " gpt read <interface> <dev>\n"
+ " gpt read <interface> <dev> [<varname>]\n"
" - read GPT into a data structure for manipulation\n"
+ " - read GPT partitions into environment variable\n"
" gpt swap <interface> <dev> <name1> <name2>\n"
" - change all partitions named name1 to name2\n"
" and vice-versa\n"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH] cmd: gpt: Add option to write GPT partitions to environment variable
2021-02-25 18:56 [PATCH] cmd: gpt: Add option to write GPT partitions to environment variable Farhan Ali
@ 2021-02-25 20:03 ` Heinrich Schuchardt
2021-02-25 22:25 ` Farhan Ali
0 siblings, 1 reply; 3+ messages in thread
From: Heinrich Schuchardt @ 2021-02-25 20:03 UTC (permalink / raw)
To: u-boot
On 2/25/21 7:56 PM, Farhan Ali wrote:
> This change would enhance the existing 'gpt read' command to allow
> (optionally) writing of the read GPT partitions to an environment
> variable. This would allow users to easily change the partition
> settings and then simply reuse the variable in the 'gpt write' and
> 'gpt verify' commands.
>
> Signed-off-by: Farhan Ali <farhan.ali@broadcom.com>
Hello Farhan,
It is unclear what your use case is.
'gpt read' already reads the data into a data structure for
manipulation. See doc/README.gpt.
Please, provide an example showing how you will use the variable.
> ---
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Corneliu Doban <cdoban@broadcom.com>
> Cc: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
>
> cmd/gpt.c | 46 ++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 38 insertions(+), 8 deletions(-)
>
> diff --git a/cmd/gpt.c b/cmd/gpt.c
> index 76a95ad..12d0551 100644
> --- a/cmd/gpt.c
> +++ b/cmd/gpt.c
> @@ -350,17 +350,46 @@ static int get_gpt_info(struct blk_desc *dev_desc)
> }
>
> /* a wrapper to test get_gpt_info */
> -static int do_get_gpt_info(struct blk_desc *dev_desc)
> +static int do_get_gpt_info(struct blk_desc *dev_desc, char * const namestr)
> {
> - int ret;
> + int numparts;
> +
> + numparts = get_gpt_info(dev_desc);
> +
> + if (numparts > 0) {
> + if (namestr) {
If the parameter is missing, the caller passes random bytes and not
NULL. So this check does not work.
> + char disk_guid[UUID_STR_LEN + 1];
> + char *partitions_list;
> + int partlistlen;
> + int ret = -1;
> +
> + ret = get_disk_guid(dev_desc, disk_guid);
> + if (ret < 0)
> + return ret;
> +
> + partlistlen = calc_parts_list_len(numparts);
> + partitions_list = malloc(partlistlen);
> + if (!partitions_list) {
> + del_gpt_info();
> + return -ENOMEM;
> + }
> + memset(partitions_list, '\0', partlistlen);
> +
> + ret = create_gpt_partitions_list(numparts, disk_guid,
> + partitions_list);
> + if (ret < 0)
> + printf("Error: Could not create partition list string!\n");
> + else
> + env_set(namestr, partitions_list);
>
> - ret = get_gpt_info(dev_desc);
> - if (ret > 0) {
> - print_gpt_info();
> + free(partitions_list);
> + } else {
> + print_gpt_info();
> + }
> del_gpt_info();
> return 0;
> }
> - return ret;
> + return numparts;
> }
> #endif
>
> @@ -982,7 +1011,7 @@ static int do_gpt(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> ret = do_disk_guid(blk_dev_desc, argv[4]);
> #ifdef CONFIG_CMD_GPT_RENAME
> } else if (strcmp(argv[1], "read") == 0) {
> - ret = do_get_gpt_info(blk_dev_desc);
> + ret = do_get_gpt_info(blk_dev_desc, argv[4]);
You have to check argc to know if argv[4] exists and pass argv[4] or
NULL accordingly.
> } else if ((strcmp(argv[1], "swap") == 0) ||
> (strcmp(argv[1], "rename") == 0)) {
> ret = do_rename_gpt_parts(blk_dev_desc, argv[1], argv[4], argv[5]);
> @@ -1028,8 +1057,9 @@ U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
> " gpt guid mmc 0 varname\n"
> #ifdef CONFIG_CMD_GPT_RENAME
> "gpt partition renaming commands:\n"
> - " gpt read <interface> <dev>\n"
> + " gpt read <interface> <dev> [<varname>]\n"
> " - read GPT into a data structure for manipulation\n"
> + " - read GPT partitions into environment variable\n"
Where is your update for doc/README.gpt?
Best regards
Heinrich
> " gpt swap <interface> <dev> <name1> <name2>\n"
> " - change all partitions named name1 to name2\n"
> " and vice-versa\n"
>
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH] cmd: gpt: Add option to write GPT partitions to environment variable
2021-02-25 20:03 ` Heinrich Schuchardt
@ 2021-02-25 22:25 ` Farhan Ali
0 siblings, 0 replies; 3+ messages in thread
From: Farhan Ali @ 2021-02-25 22:25 UTC (permalink / raw)
To: u-boot
Hi Heinrich,
Thank you for your comments. I will add your recommendations and resubmit,
I just want to clarify the use case here:
-Right now we have commands to read the GPT table into an internal data
structure, and from then on we can only rename or swap partitions
-If a user wants to modify all aspects of the partition layout, they
currently dont have any way to do so from the uboot command line
-With this proposed change, users would be able to export the entire
partition table to a uboot environment variable and then edit all of it
enmasse
Regards, Farhan
On Thu, Feb 25, 2021 at 12:03 PM Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:
> On 2/25/21 7:56 PM, Farhan Ali wrote:
> > This change would enhance the existing 'gpt read' command to allow
> > (optionally) writing of the read GPT partitions to an environment
> > variable. This would allow users to easily change the partition
> > settings and then simply reuse the variable in the 'gpt write' and
> > 'gpt verify' commands.
> >
> > Signed-off-by: Farhan Ali <farhan.ali@broadcom.com>
>
> Hello Farhan,
>
> It is unclear what your use case is.
>
> 'gpt read' already reads the data into a data structure for
> manipulation. See doc/README.gpt.
>
> Please, provide an example showing how you will use the variable.
>
> > ---
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > Cc: Corneliu Doban <cdoban@broadcom.com>
> > Cc: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> >
> > cmd/gpt.c | 46 ++++++++++++++++++++++++++++++++++++++--------
> > 1 file changed, 38 insertions(+), 8 deletions(-)
> >
> > diff --git a/cmd/gpt.c b/cmd/gpt.c
> > index 76a95ad..12d0551 100644
> > --- a/cmd/gpt.c
> > +++ b/cmd/gpt.c
> > @@ -350,17 +350,46 @@ static int get_gpt_info(struct blk_desc *dev_desc)
> > }
> >
> > /* a wrapper to test get_gpt_info */
> > -static int do_get_gpt_info(struct blk_desc *dev_desc)
> > +static int do_get_gpt_info(struct blk_desc *dev_desc, char * const
> namestr)
> > {
> > - int ret;
> > + int numparts;
> > +
> > + numparts = get_gpt_info(dev_desc);
> > +
> > + if (numparts > 0) {
> > + if (namestr) {
>
> If the parameter is missing, the caller passes random bytes and not
> NULL. So this check does not work.
>
> > + char disk_guid[UUID_STR_LEN + 1];
> > + char *partitions_list;
> > + int partlistlen;
> > + int ret = -1;
> > +
> > + ret = get_disk_guid(dev_desc, disk_guid);
> > + if (ret < 0)
> > + return ret;
> > +
> > + partlistlen = calc_parts_list_len(numparts);
> > + partitions_list = malloc(partlistlen);
> > + if (!partitions_list) {
> > + del_gpt_info();
> > + return -ENOMEM;
> > + }
> > + memset(partitions_list, '\0', partlistlen);
> > +
> > + ret = create_gpt_partitions_list(numparts,
> disk_guid,
> > + partitions_list);
> > + if (ret < 0)
> > + printf("Error: Could not create partition
> list string!\n");
> > + else
> > + env_set(namestr, partitions_list);
> >
> > - ret = get_gpt_info(dev_desc);
> > - if (ret > 0) {
> > - print_gpt_info();
> > + free(partitions_list);
> > + } else {
> > + print_gpt_info();
> > + }
> > del_gpt_info();
> > return 0;
> > }
> > - return ret;
> > + return numparts;
> > }
> > #endif
> >
> > @@ -982,7 +1011,7 @@ static int do_gpt(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const argv[])
> > ret = do_disk_guid(blk_dev_desc, argv[4]);
> > #ifdef CONFIG_CMD_GPT_RENAME
> > } else if (strcmp(argv[1], "read") == 0) {
> > - ret = do_get_gpt_info(blk_dev_desc);
> > + ret = do_get_gpt_info(blk_dev_desc, argv[4]);
>
> You have to check argc to know if argv[4] exists and pass argv[4] or
> NULL accordingly.
>
> > } else if ((strcmp(argv[1], "swap") == 0) ||
> > (strcmp(argv[1], "rename") == 0)) {
> > ret = do_rename_gpt_parts(blk_dev_desc, argv[1], argv[4],
> argv[5]);
> > @@ -1028,8 +1057,9 @@ U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
> > " gpt guid mmc 0 varname\n"
> > #ifdef CONFIG_CMD_GPT_RENAME
> > "gpt partition renaming commands:\n"
> > - " gpt read <interface> <dev>\n"
> > + " gpt read <interface> <dev> [<varname>]\n"
> > " - read GPT into a data structure for manipulation\n"
> > + " - read GPT partitions into environment variable\n"
>
> Where is your update for doc/README.gpt?
>
> Best regards
>
> Heinrich
>
> > " gpt swap <interface> <dev> <name1> <name2>\n"
> > " - change all partitions named name1 to name2\n"
> > " and vice-versa\n"
> >
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4203 bytes
Desc: S/MIME Cryptographic Signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210225/d90bba7a/attachment.bin>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-02-25 22:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-02-25 18:56 [PATCH] cmd: gpt: Add option to write GPT partitions to environment variable Farhan Ali
2021-02-25 20:03 ` Heinrich Schuchardt
2021-02-25 22:25 ` Farhan Ali
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox