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

[David Gibson]

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 
> >> ---
> >>  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


signature.asc
Description: PGP signature

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

[Serhii Popovych]

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 
>> ---
>>  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.

> 
> Regards,
> Bharata.
> 
> 


-- 
Thanks,
Serhii



signature.asc
Description: OpenPGP digital signature

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

[Bharata B Rao]

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 
> ---
>  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 ?

Regards,
Bharata.

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

[Serhii Popovych]

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 
---
 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;
-}
-
 spapr_memory_plug(hotplug_dev, dev, node, errp);
 } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
 spapr_core_plug(hotplug_dev, dev, errp);
-- 
1.8.3.1