Re: [PATCH] parted: add max option to mkpart <end>

[Maud Spierings <maud_spierings@hotmail.com>]

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