On 2/11/19 12:19 PM, Song Liu wrote:
> On Fri, Feb 8, 2019 at 10:06 PM Corey Hickey <bugfood-ml@xxxxxxxxxx> wrote:
>>
>> On 2019-01-05 22:18, bugfood-ml@xxxxxxxxxx wrote:
>>> From: Corey Hickey <bugfood-c@xxxxxxxxxx>
>>>
>>> Within the output of "mdadm --examine", there are three sizes reported
>>> on adjacent lines. For example:
>>>
>>> $ sudo mdadm --examine /dev/md3
>>> [...]
>>> Avail Dev Size : 17580545024 (8383.06 GiB 9001.24 GB)
>>> Array Size : 17580417024 (16765.99 GiB 18002.35 GB)
>>> Used Dev Size : 11720278016 (5588.66 GiB 6000.78 GB)
>>> [...]
>>>
>>> This can be confusing, since the first and third line are in 512-byte
>>> sectors, and the second is in KiB. Elsewhere, the program explicitly
>>> states "sectors" when giving values in sectors.
>>>
>>> This patch makes each value be printed in KiB and adds "KiB" to remove
>>> all ambiguity. For example:
>>>
>>> Avail Dev Size : 8790272512 KiB (8383.06 GiB 9001.24 GB)
>>> Array Size : 17580417024 KiB (16765.99 GiB 18002.35 GB)
>>> Used Dev Size : 5860139008 KiB (5588.66 GiB 6000.78 GB)
>>>
>>> (I don't particularly like the "KiB" notation, but it is at least
>>> unambiguous.)
>>>
>>> Signed-off-by: Corey Hickey <bugfood-c@xxxxxxxxxx>
>>> ---
>>> super1.c | 10 +++++-----
>>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/super1.c b/super1.c
>>> index 636a286..6b5dcb9 100644
>>> --- a/super1.c
>>> +++ b/super1.c
>>> @@ -360,8 +360,8 @@ static void examine_super1(struct supertype *st, char *homehost)
>>> printf(" Raid Level : %s\n", c?c:"-unknown-");
>>> printf(" Raid Devices : %d\n", __le32_to_cpu(sb->raid_disks));
>>> printf("\n");
>>> - printf(" Avail Dev Size : %llu%s\n",
>>> - (unsigned long long)__le64_to_cpu(sb->data_size),
>>> + printf(" Avail Dev Size : %llu KiB%s\n",
>>> + (unsigned long long)__le64_to_cpu(sb->data_size)>>1,
>>> human_size(__le64_to_cpu(sb->data_size)<<9));
>>> if (__le32_to_cpu(sb->level) > 0) {
>>> int ddsks = 0, ddsks_denom = 1;
>>> @@ -378,12 +378,12 @@ static void examine_super1(struct supertype *st, char *homehost)
>>> if (ddsks) {
>>> long long asize = __le64_to_cpu(sb->size);
>>> asize = (asize << 9) * ddsks / ddsks_denom;
>>> - printf(" Array Size : %llu%s\n",
>>> + printf(" Array Size : %llu KiB%s\n",
>>> asize >> 10, human_size(asize));
>>> }
>>> if (sb->size != sb->data_size)
>>> - printf(" Used Dev Size : %llu%s\n",
>>> - (unsigned long long)__le64_to_cpu(sb->size),
>>> + printf(" Used Dev Size : %llu KiB%s\n",
>>> + (unsigned long long)__le64_to_cpu(sb->size)>>1,
>>> human_size(__le64_to_cpu(sb->size)<<9));
>>> }
>>> if (sb->data_offset)
>>>
>>
>> Hi,
>>
>> Sorry to pester, but is this patch acceptable?
>>
>> Thanks,
>> Corey
>
> This looks good to me.
>
> Hi Jes,
>
> What do you think about it?
While being clear is a good thing, I do not think this change is the
right thing to do. There will be people having scripts in place parsing
this stuff and effectively changing the ABI will break those.
Since we already have non ambiguous reporting in GiB (yes I agree those
iB notions are utterly idiotic), I don't think it adds value to report
the same value in both GiB and KiB on the same line. If anything it
would be better to explicitly mark it as reporting in sectors.
Cheers,
Jes
