[U-Boot] [PATCH v4 0/2] Extend 'part' cmd and C API to get part info

[U-Boot] [PATCH v4 0/2] Extend 'part' cmd and C API to get part info

[Igor Opaniuk]

This patch-series extends `part` cmd and disk/part C API for getting
partition info by it's name:

1. part "number" sub-command serves for getting the partition index from
partition name. Also it can be used to test the existence of specified
partition.

2. Introduces part_get_info_by_dev_and_name_or_num() function which
allows us to get partition info from its number or name. Partition of
interest is specified by string like "device_num:partition_number" or
"device_num#partition_name".

Initially these patches were a part of
"android: implement A/B boot process" [1] patch-series, but then
due to multiple requests decided to send it separately, while the
work on A/B series is still in progress.


Use case:

For example, in Linaro Lab this U-Boot command for automatic testing
of Linux rootfs is used:

    => setenv bootpart 1:f

where 0xf is "userdata" partition. But the number of "userdata"
partition can be changed any time, when partition table is changed.

So it would be nice to get rid of that 0xf magic number and use
partition name instead, like this:

    => part number mmc 1 userdata part_num
    => setenv bootpart 1:${part_num}


[1] https://patchwork.ozlabs.org/cover/1044152/

Ruslan Trofymenko (2):
  cmd: part: Add 'number' sub-command
  disk: part: Extend API to get partition info

 cmd/part.c     | 16 +++++++++++-
 disk/part.c    | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/part.h | 21 ++++++++++++++++
 3 files changed, 104 insertions(+), 1 deletion(-)

--
2.17.1

[U-Boot] [PATCH v4 1/2] cmd: part: Add 'number' sub-command

[Igor Opaniuk]

From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>

This sub-command serves for getting the partition index from
partition name. Also it can be used to test the existence of specified
partition.

Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
Signed-off-by: Igor Opaniuk <igor.opaniuk@gmail.com>
Reviewed-by: Alistair Strachan <astrachan@google.com>
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 cmd/part.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/cmd/part.c b/cmd/part.c
index bfb6488b0f..653e13ced1 100644
--- a/cmd/part.c
+++ b/cmd/part.c
@@ -24,6 +24,7 @@
 enum cmd_part_info {
 	CMD_PART_INFO_START = 0,
 	CMD_PART_INFO_SIZE,
+	CMD_PART_INFO_NUMBER
 };
 
 static int do_part_uuid(int argc, char * const argv[])
@@ -149,6 +150,9 @@ static int do_part_info(int argc, char * const argv[], enum cmd_part_info param)
 	case CMD_PART_INFO_SIZE:
 		snprintf(buf, sizeof(buf), LBAF, info.size);
 		break;
+	case CMD_PART_INFO_NUMBER:
+		snprintf(buf, sizeof(buf), "%d", part);
+		break;
 	default:
 		printf("** Unknown cmd_part_info value: %d\n", param);
 		return 1;
@@ -172,6 +176,11 @@ static int do_part_size(int argc, char * const argv[])
 	return do_part_info(argc, argv, CMD_PART_INFO_SIZE);
 }
 
+static int do_part_number(int argc, char * const argv[])
+{
+	return do_part_info(argc, argv, CMD_PART_INFO_NUMBER);
+}
+
 static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	if (argc < 2)
@@ -185,6 +194,8 @@ static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		return do_part_start(argc - 2, argv + 2);
 	else if (!strcmp(argv[1], "size"))
 		return do_part_size(argc - 2, argv + 2);
+	else if (!strcmp(argv[1], "number"))
+		return do_part_number(argc - 2, argv + 2);
 
 	return CMD_RET_USAGE;
 }
@@ -206,5 +217,8 @@ U_BOOT_CMD(
 	"      part can be either partition number or partition name\n"
 	"part size <interface> <dev> <part> <varname>\n"
 	"    - set environment variable to the size of the partition (in blocks)\n"
-	"      part can be either partition number or partition name"
+	"      part can be either partition number or partition name\n"
+	"part number <interface> <dev> <part> <varname>\n"
+	"    - set environment variable to the partition number using the partition name\n"
+	"      part must be specified as partition name"
 );
-- 
2.17.1

[U-Boot] [PATCH v4 1/2] cmd: part: Add 'number' sub-command

[Roman Stratiienko]

On Fri, Jun 14, 2019 at 4:04 PM Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
>
> From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
>
> This sub-command serves for getting the partition index from
> partition name. Also it can be used to test the existence of specified
> partition.
>
> Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
> Signed-off-by: Igor Opaniuk <igor.opaniuk@gmail.com>
> Reviewed-by: Alistair Strachan <astrachan@google.com>
> Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
>
>  cmd/part.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/part.c b/cmd/part.c
> index bfb6488b0f..653e13ced1 100644
> --- a/cmd/part.c
> +++ b/cmd/part.c
> @@ -24,6 +24,7 @@
>  enum cmd_part_info {
>         CMD_PART_INFO_START = 0,
>         CMD_PART_INFO_SIZE,
> +       CMD_PART_INFO_NUMBER
>  };
>
>  static int do_part_uuid(int argc, char * const argv[])
> @@ -149,6 +150,9 @@ static int do_part_info(int argc, char * const argv[], enum cmd_part_info param)
>         case CMD_PART_INFO_SIZE:
>                 snprintf(buf, sizeof(buf), LBAF, info.size);
>                 break;
> +       case CMD_PART_INFO_NUMBER:
> +               snprintf(buf, sizeof(buf), "%d", part);
> +               break;
>         default:
>                 printf("** Unknown cmd_part_info value: %d\n", param);
>                 return 1;
> @@ -172,6 +176,11 @@ static int do_part_size(int argc, char * const argv[])
>         return do_part_info(argc, argv, CMD_PART_INFO_SIZE);
>  }
>
> +static int do_part_number(int argc, char * const argv[])
> +{
> +       return do_part_info(argc, argv, CMD_PART_INFO_NUMBER);
> +}
> +
>  static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  {
>         if (argc < 2)
> @@ -185,6 +194,8 @@ static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>                 return do_part_start(argc - 2, argv + 2);
>         else if (!strcmp(argv[1], "size"))
>                 return do_part_size(argc - 2, argv + 2);
> +       else if (!strcmp(argv[1], "number"))
> +               return do_part_number(argc - 2, argv + 2);
>
>         return CMD_RET_USAGE;
>  }
> @@ -206,5 +217,8 @@ U_BOOT_CMD(
>         "      part can be either partition number or partition name\n"
>         "part size <interface> <dev> <part> <varname>\n"
>         "    - set environment variable to the size of the partition (in blocks)\n"
> -       "      part can be either partition number or partition name"
> +       "      part can be either partition number or partition name\n"
> +       "part number <interface> <dev> <part> <varname>\n"
> +       "    - set environment variable to the partition number using the partition name\n"
> +       "      part must be specified as partition name"
>  );
> --
> 2.17.1
>

Hello Igor,

It would be nice if you would address Eugeniy Rosca comment (
https://patchwork.ozlabs.org/patch/1044151/ ),
and rename 'number' to 'index'

Anyway, thank you for the update,

-- 
Best regards,
Roman Stratiienko

[U-Boot] [PATCH v4 1/2] cmd: part: Add 'number' sub-command

[Igor Opaniuk]

Hi Roman,

On Fri, Jun 14, 2019 at 4:26 PM Roman Stratiienko
<roman.stratiienko@globallogic.com> wrote:
>
> On Fri, Jun 14, 2019 at 4:04 PM Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
> >
> > From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
> >
> > This sub-command serves for getting the partition index from
> > partition name. Also it can be used to test the existence of specified
> > partition.
> >
> > Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
> > Signed-off-by: Igor Opaniuk <igor.opaniuk@gmail.com>
> > Reviewed-by: Alistair Strachan <astrachan@google.com>
> > Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Changes in v4: None
> > Changes in v3: None
> > Changes in v2: None
> >
> >  cmd/part.c | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/cmd/part.c b/cmd/part.c
> > index bfb6488b0f..653e13ced1 100644
> > --- a/cmd/part.c
> > +++ b/cmd/part.c
> > @@ -24,6 +24,7 @@
> >  enum cmd_part_info {
> >         CMD_PART_INFO_START = 0,
> >         CMD_PART_INFO_SIZE,
> > +       CMD_PART_INFO_NUMBER
> >  };
> >
> >  static int do_part_uuid(int argc, char * const argv[])
> > @@ -149,6 +150,9 @@ static int do_part_info(int argc, char * const argv[], enum cmd_part_info param)
> >         case CMD_PART_INFO_SIZE:
> >                 snprintf(buf, sizeof(buf), LBAF, info.size);
> >                 break;
> > +       case CMD_PART_INFO_NUMBER:
> > +               snprintf(buf, sizeof(buf), "%d", part);
> > +               break;
> >         default:
> >                 printf("** Unknown cmd_part_info value: %d\n", param);
> >                 return 1;
> > @@ -172,6 +176,11 @@ static int do_part_size(int argc, char * const argv[])
> >         return do_part_info(argc, argv, CMD_PART_INFO_SIZE);
> >  }
> >
> > +static int do_part_number(int argc, char * const argv[])
> > +{
> > +       return do_part_info(argc, argv, CMD_PART_INFO_NUMBER);
> > +}
> > +
> >  static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >  {
> >         if (argc < 2)
> > @@ -185,6 +194,8 @@ static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >                 return do_part_start(argc - 2, argv + 2);
> >         else if (!strcmp(argv[1], "size"))
> >                 return do_part_size(argc - 2, argv + 2);
> > +       else if (!strcmp(argv[1], "number"))
> > +               return do_part_number(argc - 2, argv + 2);
> >
> >         return CMD_RET_USAGE;
> >  }
> > @@ -206,5 +217,8 @@ U_BOOT_CMD(
> >         "      part can be either partition number or partition name\n"
> >         "part size <interface> <dev> <part> <varname>\n"
> >         "    - set environment variable to the size of the partition (in blocks)\n"
> > -       "      part can be either partition number or partition name"
> > +       "      part can be either partition number or partition name\n"
> > +       "part number <interface> <dev> <part> <varname>\n"
> > +       "    - set environment variable to the partition number using the partition name\n"
> > +       "      part must be specified as partition name"
> >  );
> > --
> > 2.17.1
> >
>
> Hello Igor,
>
> It would be nice if you would address Eugeniy Rosca comment (
> https://patchwork.ozlabs.org/patch/1044151/ ),
> and rename 'number' to 'index'
>
> Anyway, thank you for the update,
>
> --
> Best regards,
> Roman Stratiienko


There is already pending discussion about re-naming "number" to "index" [1]
The main point is that it's already a long-established expression, which is used
everywhere within cmd/part.c and disk/part.c, so before introducing new
term "index", previous instances of "number" should be replaces with "index".
So renaming "number" to "index" within this patch-set will just create
more confusion,
IMHO.

[1] https://patchwork.ozlabs.org/patch/1115517/

-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk at gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk

[U-Boot] [PATCH v4 1/2] cmd: part: Add 'number' sub-command

[Eugeniu Rosca]

Hi Roman,

On Fri, Jun 14, 2019 at 3:26 PM Roman Stratiienko
<roman.stratiienko@globallogic.com> wrote:
>
> On Fri, Jun 14, 2019 at 4:04 PM Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
> > From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
[..]
> > This sub-command serves for getting the partition index from
[..]
> > +       CMD_PART_INFO_NUMBER
[..]
> > +       "part number <interface> <dev> <part> <varname>\n"
[..]
> Hello Igor,
>
> It would be nice if you would address Eugeniy Rosca comment (
> https://patchwork.ozlabs.org/patch/1044151/ ),
> and rename 'number' to 'index'

I believe the author and reviewers lean towards "partition number"
instead of "partition index" (even if the two are used intermixed in
the patch). I won't argue against that. Thanks for your comment.

-- 
Best regards,
Eugeniu.

[U-Boot] [PATCH v4 2/2] disk: part: Extend API to get partition info

[Igor Opaniuk]

From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>

This patch adds part_get_info_by_dev_and_name_or_num() function which
allows us to get partition info from its number or name. Partition of
interest is specified by string like "device_num:partition_number" or
"device_num#partition_name".

The patch was extracted from [1].

[1] https://android-review.googlesource.com/c/platform/external/u-boot/+/729880/2

Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
Signed-off-by: Igor Opaniuk <igor.opaniuk@gmail.com>
Reviewed-by: Alistair Strachan <astrachan@google.com>
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v4: None
Changes in v3: None
Changes in v2:
  * Error codes are changed to -EINVAL instead of -1

 disk/part.c    | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/part.h | 21 ++++++++++++++++
 2 files changed, 89 insertions(+)

diff --git a/disk/part.c b/disk/part.c
index 98cc54db20..580604c2bb 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -675,6 +675,74 @@ int part_get_info_by_name(struct blk_desc *dev_desc, const char *name,
 	return part_get_info_by_name_type(dev_desc, name, info, PART_TYPE_ALL);
 }
 
+/**
+ * Get partition info from device number and partition name.
+ *
+ * Parse a device number and partition name string in the form of
+ * "device_num#partition_name", for example "0#misc". If the partition
+ * is found, sets dev_desc and part_info accordingly with the information
+ * of the partition with the given partition_name.
+ *
+ * @param[in] dev_iface Device interface
+ * @param[in] dev_part_str Input string argument, like "0#misc"
+ * @param[out] dev_desc Place to store the device description pointer
+ * @param[out] part_info Place to store the partition information
+ * @return 0 on success, or a negative on error
+ */
+static int part_get_info_by_dev_and_name(const char *dev_iface,
+					 const char *dev_part_str,
+					 struct blk_desc **dev_desc,
+					 disk_partition_t *part_info)
+{
+	char *ep;
+	const char *part_str;
+	int dev_num;
+
+	part_str = strchr(dev_part_str, '#');
+	if (!part_str || part_str == dev_part_str)
+		return -EINVAL;
+
+	dev_num = simple_strtoul(dev_part_str, &ep, 16);
+	if (ep != part_str) {
+		/* Not all the first part before the # was parsed. */
+		return -EINVAL;
+	}
+	part_str++;
+
+	*dev_desc = blk_get_dev(dev_iface, dev_num);
+	if (!*dev_desc) {
+		printf("Could not find %s %d\n", dev_iface, dev_num);
+		return -EINVAL;
+	}
+	if (part_get_info_by_name(*dev_desc, part_str, part_info) < 0) {
+		printf("Could not find \"%s\" partition\n", part_str);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+int part_get_info_by_dev_and_name_or_num(const char *dev_iface,
+					 const char *dev_part_str,
+					 struct blk_desc **dev_desc,
+					 disk_partition_t *part_info)
+{
+	/* Split the part_name if passed as "$dev_num#part_name". */
+	if (!part_get_info_by_dev_and_name(dev_iface, dev_part_str,
+					   dev_desc, part_info))
+		return 0;
+	/*
+	 * Couldn't lookup by name, try looking up the partition description
+	 * directly.
+	 */
+	if (blk_get_device_part_str(dev_iface, dev_part_str,
+				    dev_desc, part_info, 1) < 0) {
+		printf("Couldn't find partition %s %s\n",
+		       dev_iface, dev_part_str);
+		return -EINVAL;
+	}
+	return 0;
+}
+
 void part_set_generic_name(const struct blk_desc *dev_desc,
 	int part_num, char *name)
 {
diff --git a/include/part.h b/include/part.h
index ebca546db5..0b5cf3d5e8 100644
--- a/include/part.h
+++ b/include/part.h
@@ -201,6 +201,27 @@ int part_get_info_by_name_type(struct blk_desc *dev_desc, const char *name,
 int part_get_info_by_name(struct blk_desc *dev_desc,
 			      const char *name, disk_partition_t *info);
 
+/**
+ * Get partition info from dev number + part name, or dev number + part number.
+ *
+ * Parse a device number and partition description (either name or number)
+ * in the form of device number plus partition name separated by a "#"
+ * (like "device_num#partition_name") or a device number plus a partition number
+ * separated by a ":". For example both "0#misc" and "0:1" can be valid
+ * partition descriptions for a given interface. If the partition is found, sets
+ * dev_desc and part_info accordingly with the information of the partition.
+ *
+ * @param[in] dev_iface	Device interface
+ * @param[in] dev_part_str Input partition description, like "0#misc" or "0:1"
+ * @param[out] dev_desc	Place to store the device description pointer
+ * @param[out] part_info Place to store the partition information
+ * @return 0 on success, or a negative on error
+ */
+int part_get_info_by_dev_and_name_or_num(const char *dev_iface,
+					 const char *dev_part_str,
+					 struct blk_desc **dev_desc,
+					 disk_partition_t *part_info);
+
 /**
  * part_set_generic_name() - create generic partition like hda1 or sdb2
  *
-- 
2.17.1

[U-Boot] [PATCH v4 0/2] Extend 'part' cmd and C API to get part info

[Igor Opaniuk]

Please ignore this patch-set,
"disk: part: Extend API to get partition info" won't be used and was
sent by by confusion.
I'll re-send a single "cmd: part: Add 'number' sub-command" as v5.

On Fri, Jun 14, 2019 at 4:04 PM Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
>
> This patch-series extends `part` cmd and disk/part C API for getting
> partition info by it's name:
>
> 1. part "number" sub-command serves for getting the partition index from
> partition name. Also it can be used to test the existence of specified
> partition.
>
> 2. Introduces part_get_info_by_dev_and_name_or_num() function which
> allows us to get partition info from its number or name. Partition of
> interest is specified by string like "device_num:partition_number" or
> "device_num#partition_name".
>
> Initially these patches were a part of
> "android: implement A/B boot process" [1] patch-series, but then
> due to multiple requests decided to send it separately, while the
> work on A/B series is still in progress.
>
>
> Use case:
>
> For example, in Linaro Lab this U-Boot command for automatic testing
> of Linux rootfs is used:
>
>     => setenv bootpart 1:f
>
> where 0xf is "userdata" partition. But the number of "userdata"
> partition can be changed any time, when partition table is changed.
>
> So it would be nice to get rid of that 0xf magic number and use
> partition name instead, like this:
>
>     => part number mmc 1 userdata part_num
>     => setenv bootpart 1:${part_num}
>
>
> [1] https://patchwork.ozlabs.org/cover/1044152/
>
> Ruslan Trofymenko (2):
>   cmd: part: Add 'number' sub-command
>   disk: part: Extend API to get partition info
>
>  cmd/part.c     | 16 +++++++++++-
>  disk/part.c    | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/part.h | 21 ++++++++++++++++
>  3 files changed, 104 insertions(+), 1 deletion(-)
>
> --
> 2.17.1
>


-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk at gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk