On Fri, 27 Mar 2020 17:51:23 +0100 David Hildenbrand <[email protected]> wrote:
> On 27.03.20 17:48, Igor Mammedov wrote: > > On Fri, 27 Mar 2020 16:29:30 +0100 > > David Hildenbrand <[email protected]> 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. > >> > >> 2. Migrating upstream QEMU to upstream QEMU (e.g., with "-m 1235M"), as > >> we receive storage attributes for memory we don't expect (as we fixed up > >> ram_size and maxram_size). > >> > >> Fixes: 3a12fc61af5c ("390x/s390-virtio-ccw: use memdev for RAM") > >> Reported-by: Lukáš Doktor <[email protected]> > >> Cc: Igor Mammedov <[email protected]> > >> Cc: Dr. David Alan Gilbert <[email protected]> > >> Signed-off-by: David Hildenbrand <[email protected]> > >> --- > >> hw/s390x/s390-skeys.c | 4 +--- > >> hw/s390x/s390-stattrib-kvm.c | 7 ++----- > >> hw/s390x/sclp.c | 21 +++++++++++---------- > >> 3 files changed, 14 insertions(+), 18 deletions(-) > >> > >> diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c > >> index 5da6e5292f..2545b1576b 100644 > >> --- a/hw/s390x/s390-skeys.c > >> +++ b/hw/s390x/s390-skeys.c > >> @@ -11,7 +11,6 @@ > >> > >> #include "qemu/osdep.h" > >> #include "qemu/units.h" > >> -#include "hw/boards.h" > >> #include "hw/s390x/storage-keys.h" > >> #include "qapi/error.h" > >> #include "qapi/qapi-commands-misc-target.h" > >> @@ -174,9 +173,8 @@ out: > >> static void qemu_s390_skeys_init(Object *obj) > >> { > >> QEMUS390SKeysState *skeys = QEMU_S390_SKEYS(obj); > >> - MachineState *machine = MACHINE(qdev_get_machine()); > >> > >> - skeys->key_count = machine->maxram_size / TARGET_PAGE_SIZE; > >> + skeys->key_count = ram_size / TARGET_PAGE_SIZE; > > > > why are you dropping machine->foo all around and switching to global > > ram_size? > > (I'd rather do other way around) > > Not sure what the latest best practice is. I can also simply convert to > machine->ram_size if that's the right thing to do. My understanding of it was not to use globals if possible. (I planned on removing global ram_size an leave only machine->ram_size but that a bit tricky since things tend to explode once a global touched, so it needs some more thought/patience)
