[PATCH] cmd: gpt: Add option to write GPT partitions to environment variable

[Farhan Ali]

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 at 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 at 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 at chromium.org>
> > Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > Cc: Corneliu Doban <cdoban at broadcom.com>
> > Cc: Rayagonda Kokatanur <rayagonda.kokatanur at 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>