Re: [PATCH v1] s390x: Reject unaligned RAM sizes

[Christian Borntraeger]

On 27.03.20 23:13, Igor Mammedov wrote:
> On Fri, 27 Mar 2020 17:53:39 +0100
> David Hildenbrand <address@hidden> wrote:
> 
>> On 27.03.20 17:46, Igor Mammedov wrote:
>>> On Fri, 27 Mar 2020 17:05:34 +0100
>>> Christian Borntraeger <address@hidden> wrote:
>>>   
>>>> On 27.03.20 17:01, David Hildenbrand wrote:  
>>>>> On 27.03.20 16:34, Christian Borntraeger wrote:    
>>>>>>
>>>>>>
>>>>>> On 27.03.20 16:29, David Hildenbrand wrote:    
>>>>>>> Historically, we fixed up the RAM size (rounded it down), to fit into
>>>>>>> storage increments. Since commit 3a12fc61af5c ("390x/s390-virtio-ccw: 
>>>>>>> use
>>>>>>> memdev for RAM"), we no longer consider the fixed-up size when
>>>>>>> allcoating the RAM block - which will break migration.
>>>>>>>
>>>>>>> Let's simply drop that manual fixup code and let the user supply sane
>>>>>>> RAM sizes. This will bail out early when trying to migrate (and make
>>>>>>> an existing guest with e.g., 12345 MB non-migratable), but maybe we
>>>>>>> should have rejected such RAM sizes right from the beginning.
>>>>>>>
>>>>>>> As we no longer fixup maxram_size as well, make other users use ram_size
>>>>>>> instead. Keep using maxram_size when setting the maximum ram size in 
>>>>>>> KVM,
>>>>>>> as that will come in handy in the future when supporting memory hotplug
>>>>>>> (in contrast, storage keys and storage attributes for hotplugged memory
>>>>>>>  will have to be migrated per RAM block in the future).
>>>>>>>
>>>>>>> This fixes (or rather rejects early):
>>>>>>>
>>>>>>> 1. Migrating older QEMU to upstream QEMU (e.g., with "-m 1235M"), as the
>>>>>>>    RAM block size changed.    
>>>>>>
>>>>>> Not sure I like this variant. Instead of breaking migration (that was 
>>>>>> accidentially done by Igors changes) we now reject migration from older
>>>>>> QEMUs to 5.0. This is not going to help those that still have such guests
>>>>>> running and want to migrate.     
>>>>>
>>>>> As Igor mentioned on another channel, you most probably can migrate an
>>>>> older guest by starting it on the target with a fixed-up size.
>>>>>
>>>>> E.g., migrate an old QEMU "-m 1235M" to a new QEMU "-m 1234M"    
>>>>
>>>> Yes, that should probably work.  
>>> I'm in process of testing it.
> 
> it works
> 
>>>   
>>>>> Not sure how many such weird-size VMs we actually do have in practice.    
>>>>
>>>> I am worried about some automated deployments where tooling has created
>>>> these sizes for dozens or hundreds of containers in VMS and so.  
>>
>> IIRC, e.g., Kata usually uses 2048MB. Not sure about others, but I'd be
>> surprised if it's not multiples of, say, 128MB.
>>
>>> Yep, it's possible but then that tooling/configs should be fixed to work 
>>> with
>>> new QEMU that validates user's input.
>>>   
>>
>> Yeah, and mention it in the cover letter, +eventually a "fixup" table
>> (e.g., old_size < X, has to be aligned to Y).
>>
>> One alternative is to have an early fixup hack in QEMU, that fixes up
>> the sizes as we did before (and eventually warns the user). Not sure if
>> we really want/need that.
> That would require at least a callback at machine level, 
> also practice shows warnings are of no use.

I would strongly prefer to not break setups that used to work and do an early 
fixup
(a machine callback). I will have a look.

> 
> If someone insist on using wrong size with new QEMU, one can write a wrapper
> script to workaround the issue (if they don't wish to fix configs/tooling).
> I don't like keeping hacks to fix user mistakes in QEMU and on top of that
> adding infrastructure for that (QEMU is already too much complicated).
> Since in this case it break migration only partially, it's sufficient
> to print error message that suggest correct size to use (like it's been done
> for other boards, s390 is the last remaining that is doing ram_size fixups).
> That way user could fix config and restart migration.
>