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

[Halil Pasic]

On Fri, 27 Mar 2020 17:46:20 +0100
Igor Mammedov <address@hidden> 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.
> 
> > > 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.
> Yep, it's possible but then that tooling/configs should be fixed to work with
> new QEMU that validates user's input.
> 

@David: I'm a little confused. Is this fix about adding user input
validation, or is it about changing what valid inputs are?

I don't see this alignment requirement documented, so my guess is the
latter. And then, I'm not sure I'm sold on this.

Regards,
Halil