Re: [PATCH v1 02/10] qemu_domain.c: align memory modules before calculating 'initialmem'
(Peter, I cc'ed you because I cited one of your commits and you might want
to weight in)
On 11/13/20 10:51 AM, Andrea Bolognani wrote:
On Fri, 2020-11-13 at 09:58 -0300, Daniel Henrique Barboza wrote:
> On 11/13/20 7:30 AM, Andrea Bolognani wrote:
>> On Wed, 2020-11-11 at 19:07 -0300, Daniel Henrique Barboza wrote:
>>> +++ b/tests/qemuxml2argvdata/memory-hotplug-ppc64-nonuma.args
[...]
I only skimmed the remaining patches and didn't dig into the code
as
much, or as recently, as you have, but from a high-level perspective
I don't see why you wouldn't be able to simply move the existing
rounding logic from the command line generator to PostParse? It's not
like the former has access to additional information that the latter
can't get to, right?
I was looking into the code and I think that might have the wrong idea here.
Apparently we're not aligning memory during migration or snapshot restore.
This specific line in qemu_command.c got my attention:
-- qemuBuildCommandLine() --
if (!migrateURI && !snapshot && qemuDomainAlignMemorySizes(def) <
0)
return NULL;
------
I investigated the history behind this logic and found the following commit:
commit c7d7ba85a6242d789ba3f4dae313e950fbb638c5
Author: Peter Krempa <pkrempa(a)redhat.com>
Date: Thu Sep 17 08:14:05 2015 +0200
qemu: command: Align memory sizes only on fresh starts
When we are starting a qemu process for an incomming migration or
snapshot reloading we should not modify the memory sizes in the domain
since we could potentially change the guest ABI that was tediously
checked before. Additionally the function now updates the initial memory
size according to the NUMA node size, which should not happen if we are
restoring state.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1252685
---------
This means that the changes made in this series will not break migration, since
the alignment of 'initialmem' is not being triggered in those cases. Which
is good.
However, you also brought up in an earlier reply that these changes might break
"the guest ABI across guest boots (if libvirt is upgraded in between them)".
This
can't be helped I think - an older ppc64 guest with 4GiB of 'currentMemory' in
the
XML, that is ending up having 4.256GiB (extra 256MiB) due to the way alignment was
being done, will have exactly 4GiB of 'initialmem' after these changes. My point
is
that we're giving the exact memory the guest is demanding, as intended by the domain
XML, in a fresh guest start. This might be considered an ABI break probably, but
why would one complain that Libvirt is now giving precisely what was asked for?
Avoiding granting extra 256MBs of mem for domains seems worth it, given that we're
not impacting live domains or migration.
Thanks,
DHB