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

[Peter Xu <peterx@redhat.com>]

On Thu, Apr 02, 2020 at 05:01:23PM +0200, Igor Mammedov wrote:
> On Thu, 2 Apr 2020 14:35:21 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
> > On 02.04.20 14:09, Christian Borntraeger wrote:
> > > 
> > > 
> > > On 02.04.20 14:05, Igor Mammedov wrote:  
> > >> On Thu, 2 Apr 2020 13:42:22 +0200
> > >> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> > >>  
> > >>> On 02.04.20 13:39, Igor Mammedov wrote:
> > >>> [...]  
> > >>>>>>       
> > >>>>>>> +                    "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.    
> > >>>>
> > >>>> MB can be dropped, since it still might not match what user specified with -m
> > >>>> it could be specified in b/kb/mb/gb over there
> > >>>>
> > >>>> so I'd drop MB and print value size_to_str() returns
> > >>>> (it will add appropriate suffix if I'm not mistaken)  
> > > 
> > > Another thing: size_to_str is also do rounding (whenever the integer part is >1000).
> > > Doesnt this result in potential messages where both numbers are the same?  
> > 
> > For example
> > 
> > 10241263616-> 9.54 GiB
> > 10241262592-> 9.54 GiB
> 
> doesn't seem to be working as one would expect (and it's used in number of places now)
> CCing original author of it

That looks sane to me.  Gib is IEC binary unit as explained by the
comment of the function, and the function prints with %0.3g so that we
keep three digits.  IIUC that's ideal when we want to display
something like disk image sizes, but for sure it won't satisfy any
formatting of digits.  Maybe add a new helper?

Thanks,

> 
> > The only guaranteed way to actually see a difference is to use MB.
> > 
> > 
> 

-- 
Peter Xu