Re: [PATCH v3 1/1] vl/s390x: fixup ram sizes for compat machines

[Christian Borntraeger]

On 02.04.20 11:27, Cornelia Huck wrote:
> On Wed, 1 Apr 2020 18:34:56 +0200
> Igor Mammedov <address@hidden> wrote:
> 
>> On Wed,  1 Apr 2020 08:37:54 -0400
>> Christian Borntraeger <address@hidden> wrote:
> 
>>> +static ram_addr_t s390_fixup_ram_size(ram_addr_t sz)
>>> +{
>>> +    /* same logic as in sclp.c */
>>> +    int increment_size = 20;
>>> +    ram_addr_t newsz;
>>> +
>>> +    while ((sz >> increment_size) > MAX_STORAGE_INCREMENTS) {
>>> +        increment_size++;
>>> +    }
>>> +    newsz = sz >> increment_size << increment_size;
>>> +
>>> +    if (sz != newsz) {
>>> +        qemu_printf("Ram size %" PRIu64 "MB was fixed up to %" PRIu64  
>>                                                    ^^^^^^^^
>>
>> for unaware  user it could be confusing as it could be read as 'value was 
>> increased'
>> s/fixed up/amended/ might be better
> 
> "rounded", perhaps?
> 
>>
>>> +                    "MB to match machine restrictions. Consider updating "
>>> +                    "the guest definition.i\n", sz / MiB, newsz / MiB);  
>>
>> also it might be better to use size_to_str() to format numbers
> 
> The text explicitly talks about 'MB'... not sure if it would be
> confusing if the user specified MB and ended up with GB or so in this
> message.
> 
>>
>>> +    }
>>> +    return newsz;
>>> +}
>>> +
> 
> (If we can agree upon message and format, I'll happily fix that up when
> applying.)

Is the the only thing that blocks this? I would rather try to get this fixed 
before rc2.