Re: [PATCH] libf2fs: reset wanted_total_sectors by new sector_size

[Chao Yu <yuchao0@huawei.com>]

On 2018/3/30 18:51, Junling Zheng wrote:
> Hi,
> 
> On 2018/3/30 17:28, Chao Yu wrote:
>> Hi All,
>>
>> On 2018/3/28 1:19, Jaegeuk Kim wrote:
>>> From: katao <katao@xiaomi.com>
>>>
>>> The args of wanted_total_sectors is calculated based
>>> on the DEFAULT_SECTOR_SIZE(512Bytes).get_device_info(i)
>>> may be reset dev_sector_size, we should reset the number
>>> of wanted_total_sectors.
>>>
>>> This bug was reported to Google Issue Tracker.
>>> Link: https://issuetracker.google.com/issues/76407663
>>
>> I don't think this is the right way, since now we have changed previous
>> sector_counter's meaning, some applications, for example, like xfstests will get
>> device's real sector size via blockdev --getsize64, then calculate total wanted
>> sector count by total_wanted_size / real_sector_size, if we changed default
>> sector size to 512bytes, xfstests will pass a wrong sector number, result in
>> getting wrong partition size.
>>
>> For something worse, in order to get the correct sector number, we have to
>> change the way of calculation method of xfstests for new mkfs, but how can
>> xfstests know the current version of mkfs is new or old...
>>
>> I think the change didn't consider backward compatibility of mkfs, so, in order
>> to keep that, we'd better to let user pass the right sector number based on
>> their device, or we can introduce a new parameter to indicate user wanted total
>> size.
>>
>> How do you think?
>>
> 
> Agree. It's not backward-compatible. Most users can pass the correct sector number
> calculated by the real sector size. For those very few users using 512B despite of
> the actual sector size, all we need to do is informing them the real sector size.

The problem is via passed sector number, we can't know user has already knew the
real sector size or not, so we don't have any chance to info them.

Thanks,

> 
> Thanks,
> Junling
> 
>> Thanks,
>>
>>>
>>> Signed-off-by: katao <katao@xiaomi.com>
>>> Signed-off-by: Jaegeuk Kim <jaegeuk@google.com>
>>> ---
>>>  lib/libf2fs.c | 9 ++++++++-
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/libf2fs.c b/lib/libf2fs.c
>>> index 0c684d5..5f11796 100644
>>> --- a/lib/libf2fs.c
>>> +++ b/lib/libf2fs.c
>>> @@ -799,8 +799,15 @@ int get_device_info(int i)
>>>  #ifdef BLKSSZGET
>>>  		if (ioctl(fd, BLKSSZGET, &sector_size) < 0)
>>>  			MSG(0, "\tError: Using the default sector size\n");
>>> -		else if (dev->sector_size < sector_size)
>>> +		else if (dev->sector_size < sector_size){
>>> +			/*
>>> +			 * wanted_total_sectors need to be reset by new
>>> +			 * sector_size.
>>> +			 */
>>> +			c.wanted_total_sectors = (c.wanted_total_sectors *
>>> +						dev->sector_size) / sector_size;
>>>  			dev->sector_size = sector_size;
>>> +		}
>>>  #endif
>>>  #ifdef BLKGETSIZE64
>>>  		if (ioctl(fd, BLKGETSIZE64, &dev->total_sectors) < 0) {
>>>
>>
>>
>> .
>>
> 
> 
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot