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

[Cornelia Huck]

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.)