Re: [PATCH v3 05/10] btrfs-progs: mkfs: align byte_count with sectorsize and zone size

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2024/5/22 16:13, Qu Wenruo 写道:
> 
> 
> 在 2024/5/22 15:32, Naohiro Aota 写道:
>> While "byte_count" is eventually rounded down to sectorsize at 
>> make_btrfs()
>> or btrfs_add_to_fs_id(), it would be better round it down first and do 
>> the
>> size checks not to confuse the things.
>>
>> Also, on a zoned device, creating a btrfs whose size is not aligned to 
>> the
>> zone boundary can be confusing. Round it down further to the zone 
>> boundary.
>>
>> The size calculation with a source directory is also tweaked to be 
>> aligned.
>>
>> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
>> ---
>>   mkfs/main.c | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/mkfs/main.c b/mkfs/main.c
>> index a437ecc40c7f..baf889873b41 100644
>> --- a/mkfs/main.c
>> +++ b/mkfs/main.c
>> @@ -1591,6 +1591,12 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>>       min_dev_size = btrfs_min_dev_size(nodesize, mixed,
>>                         opt_zoned ? zone_size(file) : 0,
>>                         metadata_profile, data_profile);
>> +    if (byte_count) {
>> +        byte_count = round_down(byte_count, sectorsize);
>> +        if (opt_zoned)
>> +            byte_count = round_down(byte_count,  zone_size(file));
>> +    }
>> +
>>       /*
>>        * Enlarge the destination file or create a new one, using the size
>>        * calculated from source dir.
>> @@ -1624,12 +1630,13 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>>            * Or we will always use source_dir_size calculated for mkfs.
>>            */
>>           if (!byte_count)
>> -            byte_count = device_get_partition_size_fd_stat(fd, 
>> &statbuf);
>> +            byte_count = 
>> round_up(device_get_partition_size_fd_stat(fd, &statbuf),
>> +                          sectorsize);

My bad, forgot this one too.

We should round_down() here.

As if we have a 512 bytes blocked device, and the partition is only 
aligned to 512 bytes, this can make the last sector go beyond device 
boundary.

Thanks,
Qu
>>           source_dir_size = btrfs_mkfs_size_dir(source_dir, sectorsize,
>>                   min_dev_size, metadata_profile, data_profile);
>>           if (byte_count < source_dir_size) {
>>               if (S_ISREG(statbuf.st_mode)) {
>> -                byte_count = source_dir_size;
>> +                byte_count = round_up(source_dir_size, sectorsize);
> 
> I believe we should round up not round down, if we're using --rootdir
> option.
> 
> As smaller size would only be more possible to hit ENOSPC.
> 
> Otherwise looks good to me.
> 
> Thanks,
> Qu
>>               } else {
>>                   warning(
>>   "the target device %llu (%s) is smaller than the calculated source 
>> directory size %llu (%s), mkfs may fail",
>