* [PATCH] parted: add max option to mkpart <end>
@ 2025-10-19 15:07 Maud Spierings via B4 Relay
2025-10-20 8:16 ` Sascha Hauer
0 siblings, 1 reply; 3+ messages in thread
From: Maud Spierings via B4 Relay @ 2025-10-19 15:07 UTC (permalink / raw)
To: Sascha Hauer, BAREBOX; +Cc: Maud Spierings
From: Maud Spierings <maud_spierings@hotmail.com>
Add the option to specify "max" as the end location, this will fill the
block device up to the end of its available space.
A secondary effect is that it is now possible to use two different size
units for start and end
mkpart root ext4 66MiB 3866111KiB
previously it would read 66MiB as 66KiB as it only used the last read
unit.
Signed-off-by: Maud Spierings <maud_spierings@hotmail.com>
---
At first it was considered to make <end> an optional argument. But this
would make chaining parted subcommands very difficult to manage.
Therefore I chose to make "max" an option so this can still be done like
so:
parted /dev/mmc0 mklabel gpt mkpart env bbenv 1MiB 2MiB mkpart boot fat32 2MiB 66MiB mkpart root ext4 66MiB max print
I get a checkpatch error about assignment in an if statement, the issue
is that there is an error print in parted_strtoull(), so if it checks
for a regular value but it is "max" it will print an error even though
there is none.
removing that print and adding it to every place where it is used seems
excessive too. What would be a good alternative approach?
Another option I have considered is making end=0 mean fill to the end,
if that is preferred I will make it so in v2
---
commands/parted.c | 56 +++++++++++++++++++++++++++++++++++--------------------
1 file changed, 36 insertions(+), 20 deletions(-)
diff --git a/commands/parted.c b/commands/parted.c
index 7ec56da4c15f..0e858eb881ba 100644
--- a/commands/parted.c
+++ b/commands/parted.c
@@ -138,6 +138,10 @@ static int do_mkpart(struct block_device *blk, int argc, char *argv[])
int ret;
uint64_t mult;
+ pdesc = pdesc_get(blk);
+ if (!pdesc)
+ return -EINVAL;
+
if (argc < 5) {
printf("Error: Missing required arguments\n");
return -EINVAL;
@@ -150,40 +154,51 @@ static int do_mkpart(struct block_device *blk, int argc, char *argv[])
if (ret)
return ret;
- ret = parted_strtoull(argv[4], &end, &mult);
- if (ret)
- return ret;
-
if (!mult)
mult = gunit;
-
start *= mult;
- end *= mult;
- /* If not on sector boundaries move start up and end down */
+ /* If not on sector boundaries round start up */
start = ALIGN(start, SZ_1M);
- end = ALIGN_DOWN(end, SZ_1M);
/* convert to LBA */
start >>= SECTOR_SHIFT;
- end >>= SECTOR_SHIFT;
+
+ if (!strcmp(argv[4], "max")) {
+ /* gpt requires 34 blocks at the end */
+ if (pdesc->parser->type == filetype_gpt)
+ end = blk->num_blocks - 35;
+ else if (pdesc->parser->type == filetype_mbr)
+ end = blk->num_blocks - 1;
+ else
+ return -ENOSYS;
+ } else if (!(ret = parted_strtoull(argv[4], &end, &mult))) {
+ if (!mult)
+ mult = gunit;
+ end *= mult;
+
+ /* If not on sector boundaries round end down */
+ end = ALIGN_DOWN(end, SZ_1M);
+
+ /* convert to LBA */
+ end >>= SECTOR_SHIFT;
+
+ /*
+ * When unit is >= KB then subtract one sector for user
+ * convenience. It allows to start the next partition where the
+ * previous ends
+ */
+ if (mult >= 1000)
+ end -= 1;
+ } else {
+ return ret;
+ }
if (end == start) {
printf("Error: After alignment the partition has zero size\n");
return -EINVAL;
}
- /*
- * When unit is >= KB then substract one sector for user convenience.
- * It allows to start the next partition where the previous ends
- */
- if (mult >= 1000)
- end -= 1;
-
- pdesc = pdesc_get(blk);
- if (!pdesc)
- return -EINVAL;
-
ret = partition_create(pdesc, name, fs_type, start, end);
if (!ret)
@@ -424,6 +439,7 @@ BAREBOX_CMD_HELP_TEXT("<type> must be \"gpt\" or \"msdos\"")
BAREBOX_CMD_HELP_TEXT("<fstype> can be one of \"ext2\", \"ext3\", \"ext4\", \"fat16\", \"fat32\" or \"bbenv\"")
BAREBOX_CMD_HELP_TEXT("<name> for MBR partition tables can be one of \"primary\", \"extended\" or")
BAREBOX_CMD_HELP_TEXT("\"logical\". For GPT this is a name string.")
+BAREBOX_CMD_HELP_TEXT("<end> can be \"max\" it will take all remaining space")
BAREBOX_CMD_HELP_END
BAREBOX_CMD_START(parted)
---
base-commit: 8defba1d0ab1aef9dd5d57710e18d0d02e2c48e2
change-id: 20251019-parted-b0d637580e80
Best regards,
--
Maud Spierings <maud_spierings@hotmail.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] parted: add max option to mkpart <end>
2025-10-19 15:07 [PATCH] parted: add max option to mkpart <end> Maud Spierings via B4 Relay
@ 2025-10-20 8:16 ` Sascha Hauer
2025-10-20 8:23 ` Maud Spierings
0 siblings, 1 reply; 3+ messages in thread
From: Sascha Hauer @ 2025-10-20 8:16 UTC (permalink / raw)
To: Maud Spierings via B4 Relay; +Cc: BAREBOX, Maud Spierings
Hi Maud,
On Sun, Oct 19, 2025 at 05:07:06PM +0200, Maud Spierings via B4 Relay wrote:
> From: Maud Spierings <maud_spierings@hotmail.com>
>
> Add the option to specify "max" as the end location, this will fill the
> block device up to the end of its available space.
>
> A secondary effect is that it is now possible to use two different size
> units for start and end
>
> mkpart root ext4 66MiB 3866111KiB
>
> previously it would read 66MiB as 66KiB as it only used the last read
> unit.
Uh, yes, that's a bug. Thanks for fixing this.
> diff --git a/commands/parted.c b/commands/parted.c
> index 7ec56da4c15f..0e858eb881ba 100644
> --- a/commands/parted.c
> +++ b/commands/parted.c
> @@ -138,6 +138,10 @@ static int do_mkpart(struct block_device *blk, int argc, char *argv[])
> int ret;
> uint64_t mult;
>
> + pdesc = pdesc_get(blk);
> + if (!pdesc)
> + return -EINVAL;
> +
> if (argc < 5) {
> printf("Error: Missing required arguments\n");
> return -EINVAL;
> @@ -150,40 +154,51 @@ static int do_mkpart(struct block_device *blk, int argc, char *argv[])
> if (ret)
> return ret;
>
> - ret = parted_strtoull(argv[4], &end, &mult);
> - if (ret)
> - return ret;
> -
> if (!mult)
> mult = gunit;
> -
> start *= mult;
> - end *= mult;
>
> - /* If not on sector boundaries move start up and end down */
> + /* If not on sector boundaries round start up */
> start = ALIGN(start, SZ_1M);
> - end = ALIGN_DOWN(end, SZ_1M);
I am not sure we need the alignment of the end at all. I can imagine
that it helps the storage device to align the start of a partition, but
the end shouldn't really matter.
Maybe we should remove the end alignment because with this patch the
ending becomes inconsistent. It will align down to 1MiB boundary when
the size is specified, but will use an unaligned ending with size =
"max".
Better drop the end alignment entirely for more consistency.
>
> /* convert to LBA */
> start >>= SECTOR_SHIFT;
> - end >>= SECTOR_SHIFT;
> +
> + if (!strcmp(argv[4], "max")) {
> + /* gpt requires 34 blocks at the end */
> + if (pdesc->parser->type == filetype_gpt)
> + end = blk->num_blocks - 35;
> + else if (pdesc->parser->type == filetype_mbr)
> + end = blk->num_blocks - 1;
> + else
> + return -ENOSYS;
> + } else if (!(ret = parted_strtoull(argv[4], &end, &mult))) {
Rewrite to:
} else {
ret = parted_strtoull(argv[4], &end, &mult);
if (ret)
return ret;
...
}
It solves the checkpatch warning and is easier to read IMO.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] parted: add max option to mkpart <end>
2025-10-20 8:16 ` Sascha Hauer
@ 2025-10-20 8:23 ` Maud Spierings
0 siblings, 0 replies; 3+ messages in thread
From: Maud Spierings @ 2025-10-20 8:23 UTC (permalink / raw)
To: Sascha Hauer, Maud Spierings via B4 Relay; +Cc: BAREBOX
Hi Sascha,
On 10/20/25 10:16, Sascha Hauer wrote:
> Hi Maud,
>
> On Sun, Oct 19, 2025 at 05:07:06PM +0200, Maud Spierings via B4 Relay wrote:
>> From: Maud Spierings <maud_spierings@hotmail.com>
>>
>> Add the option to specify "max" as the end location, this will fill the
>> block device up to the end of its available space.
>>
>> A secondary effect is that it is now possible to use two different size
>> units for start and end
>>
>> mkpart root ext4 66MiB 3866111KiB
>>
>> previously it would read 66MiB as 66KiB as it only used the last read
>> unit.
>
> Uh, yes, that's a bug. Thanks for fixing this.
>
>> diff --git a/commands/parted.c b/commands/parted.c
>> index 7ec56da4c15f..0e858eb881ba 100644
>> --- a/commands/parted.c
>> +++ b/commands/parted.c
>> @@ -138,6 +138,10 @@ static int do_mkpart(struct block_device *blk, int argc, char *argv[])
>> int ret;
>> uint64_t mult;
>>
>> + pdesc = pdesc_get(blk);
>> + if (!pdesc)
>> + return -EINVAL;
>> +
>> if (argc < 5) {
>> printf("Error: Missing required arguments\n");
>> return -EINVAL;
>> @@ -150,40 +154,51 @@ static int do_mkpart(struct block_device *blk, int argc, char *argv[])
>> if (ret)
>> return ret;
>>
>> - ret = parted_strtoull(argv[4], &end, &mult);
>> - if (ret)
>> - return ret;
>> -
>> if (!mult)
>> mult = gunit;
>> -
>> start *= mult;
>> - end *= mult;
>>
>> - /* If not on sector boundaries move start up and end down */
>> + /* If not on sector boundaries round start up */
>> start = ALIGN(start, SZ_1M);
>> - end = ALIGN_DOWN(end, SZ_1M);
>
> I am not sure we need the alignment of the end at all. I can imagine
> that it helps the storage device to align the start of a partition, but
> the end shouldn't really matter.
>
> Maybe we should remove the end alignment because with this patch the
> ending becomes inconsistent. It will align down to 1MiB boundary when
> the size is specified, but will use an unaligned ending with size =
> "max".
>
> Better drop the end alignment entirely for more consistency.>>
>> /* convert to LBA */
>> start >>= SECTOR_SHIFT;
>> - end >>= SECTOR_SHIFT;
>> +
>> + if (!strcmp(argv[4], "max")) {
>> + /* gpt requires 34 blocks at the end */
>> + if (pdesc->parser->type == filetype_gpt)
>> + end = blk->num_blocks - 35;
>> + else if (pdesc->parser->type == filetype_mbr)
>> + end = blk->num_blocks - 1;
>> + else
>> + return -ENOSYS;
>> + } else if (!(ret = parted_strtoull(argv[4], &end, &mult))) {
>
> Rewrite to:
>
> } else {
> ret = parted_strtoull(argv[4], &end, &mult);
> if (ret)
> return ret;
> ...
> }
>
> It solves the checkpatch warning and is easier to read IMO.
>
Will do, thanks for the review!
Kind regards,
Maud
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-10-20 16:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-19 15:07 [PATCH] parted: add max option to mkpart <end> Maud Spierings via B4 Relay
2025-10-20 8:16 ` Sascha Hauer
2025-10-20 8:23 ` Maud Spierings
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox