Re: [PATCH v3 2/8] doc: Add gpt command documentation

Joshua Watt
2023-08-28T21:02:20Z

On Mon, Aug 28, 2023 at 2:00 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 8/28/23 21:29, Joshua Watt wrote:
> > On Fri, Aug 25, 2023 at 7:57 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>
> >> On 8/25/23 21:38, Joshua Watt wrote:
> >>> Adds initial documentation for the gpt command
> >>
> >> Thanks a lot for filling the gap.
> >>
> >>>
> >>> Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
> >>> ---
> >>> doc/usage/cmd/gpt.rst | 141 ++++++++++++++++++++++++++++++++++++++++++
> >>> doc/usage/index.rst | 1 +
> >>> 2 files changed, 142 insertions(+)
> >>> create mode 100644 doc/usage/cmd/gpt.rst
> >>>
> >>> diff --git a/doc/usage/cmd/gpt.rst b/doc/usage/cmd/gpt.rst
> >>> new file mode 100644
> >>> index 0000000000..f6e082fb94
> >>> --- /dev/null
> >>> +++ b/doc/usage/cmd/gpt.rst
> >>> @@ -0,0 +1,141 @@
> >>> +.. SPDX-License-Identifier: GPL-2.0+
> >>> +
> >>> +gpt command
> >>> +===========
> >>> +
> >>> +Synopsis
> >>> +--------
> >>> +
> >>> +::
> >>> +
> >>> + gpt repair <interface> <device no>
> >>> + gpt write <interface> <device no> <partition string>
> >>> + gpt verify <interface> <device no> <partition string>
> >>
> >> <partition string> is not required. Please, add brackets [].
> >
> > It's not optional; there is no standardized string for the list of GPT
> > partitions like there is for MBR
>
> You can use the command without partition string. It will tell you if
> the device is partitioned or not.

Ah, right. I missed that

>
> >
> >>
> >>> + gpt setenv <interface> <device no> <partition name>
> >>
> >> gpt setenv can be called without partition name which leads to a crash.
> >>
> >> If that parameter is meant to be optional, this should be indicated
> >> here. Otherwise an argc check is missing.
> >
> > I don't believe the argument is optional, but either way it can be
> > fixed in a subsequent patch series. I'd rather not hold up the
> > documentation to fix bugs
> >
> >>
> >>> + gpt enumerate <interface> <device no>
> >>> + gpt guid <interface> <device no> [<varname>]
> >>> + gpt read <interface> <device no> [<varname>]
> >>> + gpt swap <interface> <dev> <name1> <name2>
> >>> + gpt rename <interface> <dev> <part> <name>
> >>
> >> Thanks a lot for providing a man-page for this command.
> >>
> >> If <device no> and <dev> both relate to the same object type, we should
> >> use the same text. <dev> would be fine.
> >>
> >> The sequence looks random. Please, sort the sub-commands either
> >> logically or alphabetically.
> >
> > They are sorted to match the gpt command help text. Reordering of both
> > can be done later if desired.
>
> There is no requirement to follow the gpt help =sequence here.
>
> We should not reproduce what is not helpful.
>
> Best regards
>
> Heinrich
>
> >
> >>
> >>> +
> >>> +Description
> >>> +-----------
> >>> +
> >>> +The gpt command lets users read, create, modify, or verify the GPT (GUID
> >>> +Partition Table) partition layout.
> >>> +
> >>> +The syntax of the text description of the partition list is similar to
> >>> +the one used by the 'mbr' command. The string contains one or more partition
> >>
> >> Please, link the mbr page:
> >>
> >> by the :doc:`mbr command <mbr>`.
> >>
> >>> +descriptors, each separated by a ";". Each descriptor contains one or more
> >>> +fields, with each field separated by a ",". Fields are either of the form
> >>> +"key=value" to set a specific value, or simple "flag" to set a boolean flag
> >>
> >> At this point it remains unclear to the reader what this ;-separated
> >> string format relates to.
> >> Is it an output format?
> >> Is it a format used in variables?
> >> Is it used for the parameter 'partition string'?
> >>
> >> Maybe describe it after the parameters and relate it to the 'partition
> >> string' parameter.
> >>
> >> Please, describe all parameters (in this indented format):
> >>
> >> interface
> >> interface for accessing the block device (mmc, sata, scsi, usb, ....)
> >>
> >> device no
> >> device number
> >>
> >> ...
> >>
> >>> +
> >>> +The first descriptor can optionally be used to describe parameters for the
> >>> +whole disk with the following fields:
> >>> +
> >>> +* uuid_disk=UUID - Set the UUID for the disk
> >>> +
> >>> +Partition descriptors can have the following fields:
> >>> +* name=NAME - The partition name, required
> >>
> >> Maybe better
> >>
> >> name=<NAME>
> >>
> >> This is not rendered as an unordered list but everything is in one line.
> >> Please, add the missing blank lines.
> >>
> >> Please, generate the HTML documentation as described in
> >> https://u-boot.readthedocs.io/en/latest/build/documentation.html
> >> and check the output before resubmitting.
> >>
> >>> +* start=BYTES - The partition start offset in bytes, required
> >>> +* size=BYTES - The partition size, in bytes or "-" to expand it to the whole free area
> >>> +* bootable - Set the legacy bootable flag
> >>> +* uuid=UUID - Set the partition UUID, optional if CONFIG_RANDOM_UUID=y is enabled
> >>> +* type=UUID - Set the partition type GUID, requires CONFIG_PARTITION_TYPE_GUID=y
> >>> +
> >>
> >> The following should be in a separate 'Examples' section to match the
> >> other man-pages.
> >>
> >>> +Here is an example how to create a 6 partitions, some of the predefined sizes:
> >>> +
> >>> +::
> >>> +
> >>> + => setenv gpt_parts 'uuid_disk=bec9fc2a-86c1-483d-8a0e-0109732277d7;
> >>> + name=boot,start=4M,size=128M,bootable,type=ebd0a0a2-b9e5-4433-87c0-68b6b72699c7,
> >>> + name=rootfs,size=3072M,type=0fc63daf-8483-4772-8e79-3d69d8477de4;
> >>> + name=system-data,size=512M,type=0fc63daf-8483-4772-8e79-3d69d8477de4;
> >>> + name=[ext],size=-,type=0fc63daf-8483-4772-8e79-3d69d8477de4;
> >>> + name=user,size=-,type=0fc63daf-8483-4772-8e79-3d69d8477de4;
> >>> + name=modules,size=100M,type=0fc63daf-8483-4772-8e79-3d69d8477de4;
> >>> + name=ramdisk,size=8M,type=0fc63daf-8483-4772-8e79-3d69d8477de4
> >>> + => gpt write mmc 0 $gpt_parts
> >>> +
> >>> +
> >>> +If 'uuid' is not specified, but CONFIG_RANDOM_UUID is enabled, a random UUID
> >>> +will be generated for the partition
> >>> +
> >>> +The 'gpt verify' command returns 0 if the layout matches the one on the storage
> >>> +device or 1 if not. To check if the layout on the MMC #0 storage device
> >>> +matches the provided text description one has to issue following command:
> >>
> >> The command can be used with and without partition parameter. This
> >> should be described here.
> >>
> >> I would prefer:
> >>
> >> sets the return value $? to 0 (true) if the layout
> >> or 1 (false) if not
> >>
> >>> +
> >>> +::
> >>> +
> >>> + => gpt verify mmc 0 $gpt_parts
> >>> +
> >>> +The verify sub-command is especially useful in the system update scripts:
> >>> +
> >>> +::
> >>> +
> >>> + => if gpt verify mmc 0 $gpt_parts; then
> >>> + echo GPT layout needs to be updated
> >>> + ...
> >>> + fi
> >>> +
> >>> +The 'gpt write' command returns 0 on success write or 1 on failure.
> >>> +
> >>> +The 'gpt setenv' command will set a series of environment variables with
> >>> +information about a particular partition. The variables are:
> >>> +
> >>> +* gpt_partition_addr (the starting offset of the partition, in hexadecimal blocks)
> >>> +* gpt_partition_size (the size of the partition, in hexadecimal blocks)
> >>
> >> The blocks that are not hexadecimal, the numbers are.
> >>
> >> gpt_partition_addr (first block of the partition as hexadecimal number)
> >> gpt_partition_size (number of blocks as hexadecimal number)
> >>
> >>> +* gpt_partition_name (the name of the partition)
> >>> +* gpt_partition_entry (the partition number in the table, e.g. 1, 2, 3, etc.)
> >>
> >> This value is currently decimal but this seems to be a bug. We need a
> >> hexadecimal number to use this variable in a command like
> >>
> >> load host 0:$gpt_partition_entry $loadaddr filename
> >>
> >> Another bug is that this command assumes that partitions are
> >> continuously numbered.
> >>
> >>> +
> >>> +To get the information about the partition named 'rootfs', issue the following
> >>> +command:
> >>> +
> >>> +::
> >>> + => gpt setenv mmc 0 rootfs
> >>> + => echo ${gpt_partition_addr}
> >>> + 2000
> >>> + => echo ${gpt_partition_size}
> >>> + 14a000
> >>> + => echo ${gpt_partition_name}
> >>> + rootfs
> >>> + => echo ${gpt_partition_entry}
> >>> + 2
> >>> +
> >>> +The 'gpt enumerate' command will set the variable 'gpt_partition_list' with the
> >>> +list of partition names on the device. For example:
> >>> +
> >>> +::
> >>> + => gpt enumerate
> >>> + => echo gpt_partition_list
> >>> + boot rootfs system-data [ext] user modules ramdisk
> >>
> >> As we cannot see from the output if there is a partition 'user' and a
> >> partition 'modules' or only a single partition 'user modules' this
> >> sub-command seems to need some rework.
> >
> > Can be fixed later.
> >
> >>
> >> Best regards
> >>
> >> Heinrich
> >>
> >>> +
> >>> +The 'gpt guid' command will report the GUID of a disk. If 'varname' is
> >>> +specified, the command will set the variable to the GUID, otherwise it will be
> >>> +printed out. For example:
> >>> +
> >>> +::
> >>> + => gpt guid mmc 0
> >>> + bec9fc2a-86c1-483d-8a0e-0109732277d7
> >>> + => gpt guid mmc gpt_disk_uuid
> >>> + => echo ${gpt_disk_uuid}
> >>> + bec9fc2a-86c1-483d-8a0e-0109732277d7
> >>> +
> >>> +The 'gpt read' command will print out the current state of the GPT partition
> >>> +table. If 'varname' is specified, the variable will be filled with a partition
> >>> +string as described above that is suitable for passing to other 'gpt' commands.
> >>> +If omitted, a human readable description is printed out.
> >>> +CONFIG_CMD_GPT_RENAME=y is required.
> >>> +
> >>> +The 'gpt swap' command changes the names of all partitions that are named
> >>> +'name1' to be 'name2', and all partitions named 'name2' to be 'name1'.
> >>> +CONFIG_CMD_GPT_RENAME=y is required.
> >>> +
> >>> +The 'gpt rename' command renames all partitions named 'part' to be 'name1'.
> >>> +CONFIG_CMD_GPT_RENAME=y is required.
> >>> +
> >>> +Configuration
> >>> +-------------
> >>> +
> >>> +To use the 'gpt' command you must specify CONFIG_CMD_GPT=y. To enable 'gpt
> >>> +read', 'gpt swap' and 'gpt rename', you must specify CONFIG_CMD_GPT_RENAME=y.
> >>> diff --git a/doc/usage/index.rst b/doc/usage/index.rst
> >>> index 3326ec82ff..4bfaabbd16 100644
> >>> --- a/doc/usage/index.rst
> >>> +++ b/doc/usage/index.rst
> >>> @@ -65,6 +65,7 @@ Shell commands
> >>> cmd/for
> >>> cmd/fwu_mdata
> >>> cmd/gpio
> >>> + cmd/gpt
> >>> cmd/host
> >>> cmd/imxtract
> >>> cmd/load
> >>
>