Re: [Qemu-devel] [PATCH for 2.13 1/2] Revert "spapr: Don't allow memory hotplug to memory less nodes"

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3081 bytes --]

On Fri, Apr 06, 2018 at 08:48:55AM +0300, Serhii Popovych wrote:
> Bharata B Rao wrote:
> > On Thu, Apr 05, 2018 at 10:35:22AM -0400, Serhii Popovych wrote:
> >> This reverts commit b556854bd8524c26b8be98ab1bfdf0826831e793.
> >>
> >> Leave change @node type from uint32_t to to int from reverted commit
> >> because node < 0 is always false.
> >>
> >> Signed-off-by: Serhii Popovych <spopovyc@redhat.com>
> >> ---
> >>  hw/ppc/spapr.c | 22 ----------------------
> >>  1 file changed, 22 deletions(-)
> >>
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index 2c0be8c..3ad4545 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -3477,28 +3477,6 @@ static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> >>              return;
> >>          }
> >>
> >> -        /*
> >> -         * Currently PowerPC kernel doesn't allow hot-adding memory to
> >> -         * memory-less node, but instead will silently add the memory
> >> -         * to the first node that has some memory. This causes two
> >> -         * unexpected behaviours for the user.
> >> -         *
> >> -         * - Memory gets hotplugged to a different node than what the user
> >> -         *   specified.
> >> -         * - Since pc-dimm subsystem in QEMU still thinks that memory belongs
> >> -         *   to memory-less node, a reboot will set things accordingly
> >> -         *   and the previously hotplugged memory now ends in the right node.
> >> -         *   This appears as if some memory moved from one node to another.
> >> -         *
> >> -         * So until kernel starts supporting memory hotplug to memory-less
> >> -         * nodes, just prevent such attempts upfront in QEMU.
> >> -         */
> >> -        if (nb_numa_nodes && !numa_info[node].node_mem) {
> >> -            error_setg(errp, "Can't hotplug memory to memory-less node %d",
> >> -                       node);
> >> -            return;
> >> -        }
> >> -
> > 
> > If you remove this unconditionally, wouldn't it be a problem in case
> > of newer QEMU with older guest kernels ?
> 
> Yes, that definitely would affect guest kernels without such support. We
> probably need to add some capability to test for guest kernel
> functionality presence.

Hm, maybe.

So first, we should check when the guest side support came in.  If
it's old enough we might not care.

PAPR does include a mechanism for negotiating guest/host
capabilities.  However, I don't think it has a bit for this specific
feature, so I can't really see a way to do this cleanly.

I don't think we necessarily have to handle that case: it's not like
we can reasonably workaround *every* possible guest bug/limitation
from the host side.  If you want to create a system with a memory-less
node you need an OS that can handle that, nothing really special there.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]