[Qemu-devel] [PATCH] numa: warn if numa 'mem' option or default RAM splitting between nodes is used.

[Qemu-devel] [PATCH] numa: warn if numa 'mem' option or default RAM splitting between nodes is used.

[Igor Mammedov]

Ammend -numa option docs and print warnings if 'mem' option or default RAM
splitting between nodes is used. It's intended to discourage users from using
configuration that allows only to fake NUMA on guest side while leading
to reduced performance of the guest due to inability to properly configure
VM's RAM on the host.

In NUMA case, it's recommended to always explicitly configure guest RAM
using -numa node,memdev={backend-id} option.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 numa.c          |  5 +++++
 qemu-options.hx | 12 ++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/numa.c b/numa.c
index 3875e1e..c6c2a6f 100644
--- a/numa.c
+++ b/numa.c
@@ -121,6 +121,8 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
 
     if (node->has_mem) {
         numa_info[nodenr].node_mem = node->mem;
+        warn_report("Parameter -numa node,mem is obsolete,"
+                    " use -numa node,memdev instead");
     }
     if (node->has_memdev) {
         Object *o;
@@ -407,6 +409,9 @@ void numa_complete_configuration(MachineState *ms)
         if (i == nb_numa_nodes) {
             assert(mc->numa_auto_assign_ram);
             mc->numa_auto_assign_ram(mc, numa_info, nb_numa_nodes, ram_size);
+            warn_report("Default splitting of RAM between nodes is obsolete,"
+                        " Use '-numa node,memdev' to explictly define RAM"
+                        " allocation per node");
         }
 
         numa_total = 0;
diff --git a/qemu-options.hx b/qemu-options.hx
index 1cf9aac..033e311 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -206,10 +206,14 @@ For example:
 -numa cpu,node-id=0,socket-id=0 -numa cpu,node-id=1,socket-id=1
 @end example
 
-@samp{mem} assigns a given RAM amount to a node. @samp{memdev}
-assigns RAM from a given memory backend device to a node. If
-@samp{mem} and @samp{memdev} are omitted in all nodes, RAM is
-split equally between them.
+@samp{memdev} assigns RAM from a given memory backend device to a node.
+
+Legacy options/behaviour: @samp{mem} assigns a given RAM amount to a node.
+If @samp{mem} and @samp{memdev} are omitted in all nodes, RAM is split equally
+between them. Option @samp{mem} and default RAM splitting are obsolete as they
+do not provide means to manage RAM on host side and only allow QEMU to fake
+NUMA support which in practice could degrade VM performance.
+It's advised to always explicitly configure NUMA RAM using @samp{memdev} option.
 
 @samp{mem} and @samp{memdev} are mutually exclusive. Furthermore,
 if one node uses @samp{memdev}, all of them have to use it.
-- 
2.7.4

Re: [Qemu-devel] [PATCH] numa: warn if numa 'mem' option or default RAM splitting between nodes is used.

[Eric Blake]

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

On 3/6/19 10:30 AM, Igor Mammedov wrote:
> Ammend -numa option docs and print warnings if 'mem' option or default RAM

s/Ammend/Amend/

> splitting between nodes is used. It's intended to discourage users from using
> configuration that allows only to fake NUMA on guest side while leading
> to reduced performance of the guest due to inability to properly configure
> VM's RAM on the host.
> 
> In NUMA case, it's recommended to always explicitly configure guest RAM
> using -numa node,memdev={backend-id} option.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  numa.c          |  5 +++++
>  qemu-options.hx | 12 ++++++++----
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 

> @@ -407,6 +409,9 @@ void numa_complete_configuration(MachineState *ms)
>          if (i == nb_numa_nodes) {
>              assert(mc->numa_auto_assign_ram);
>              mc->numa_auto_assign_ram(mc, numa_info, nb_numa_nodes, ram_size);
> +            warn_report("Default splitting of RAM between nodes is obsolete,"
> +                        " Use '-numa node,memdev' to explictly define RAM"

s/explictly/explicitly/


> +++ b/qemu-options.hx
> @@ -206,10 +206,14 @@ For example:
>  -numa cpu,node-id=0,socket-id=0 -numa cpu,node-id=1,socket-id=1
>  @end example
>  
> -@samp{mem} assigns a given RAM amount to a node. @samp{memdev}
> -assigns RAM from a given memory backend device to a node. If
> -@samp{mem} and @samp{memdev} are omitted in all nodes, RAM is
> -split equally between them.
> +@samp{memdev} assigns RAM from a given memory backend device to a node.
> +
> +Legacy options/behaviour: @samp{mem} assigns a given RAM amount to a node.
> +If @samp{mem} and @samp{memdev} are omitted in all nodes, RAM is split equally
> +between them. Option @samp{mem} and default RAM splitting are obsolete as they
> +do not provide means to manage RAM on host side and only allow QEMU to fake

s/on host/on the host/

> +NUMA support which in practice could degrade VM performance.
> +It's advised to always explicitly configure NUMA RAM using @samp{memdev} option.

s/using/by using the/

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH] numa: warn if numa 'mem' option or default RAM splitting between nodes is used.

[Daniel P. Berrangé]

On Wed, Mar 06, 2019 at 05:30:25PM +0100, Igor Mammedov wrote:
> Ammend -numa option docs and print warnings if 'mem' option or default RAM
> splitting between nodes is used. It's intended to discourage users from using
> configuration that allows only to fake NUMA on guest side while leading
> to reduced performance of the guest due to inability to properly configure
> VM's RAM on the host.
> 
> In NUMA case, it's recommended to always explicitly configure guest RAM
> using -numa node,memdev={backend-id} option.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  numa.c          |  5 +++++
>  qemu-options.hx | 12 ++++++++----
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/numa.c b/numa.c
> index 3875e1e..c6c2a6f 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -121,6 +121,8 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
>  
>      if (node->has_mem) {
>          numa_info[nodenr].node_mem = node->mem;
> +        warn_report("Parameter -numa node,mem is obsolete,"
> +                    " use -numa node,memdev instead");

I don't think we should do this. Libvirt isn't going to stop using this
option in the near term. When users see warnings like this in logs
they'll often file bugs reports thinking something is broken which is
not the case here. 

>      }
>      if (node->has_memdev) {
>          Object *o;
> @@ -407,6 +409,9 @@ void numa_complete_configuration(MachineState *ms)
>          if (i == nb_numa_nodes) {
>              assert(mc->numa_auto_assign_ram);
>              mc->numa_auto_assign_ram(mc, numa_info, nb_numa_nodes, ram_size);
> +            warn_report("Default splitting of RAM between nodes is obsolete,"
> +                        " Use '-numa node,memdev' to explictly define RAM"
> +                        " allocation per node");

I think this one is ok as I don't believe libvirt triggers this code path

> diff --git a/qemu-options.hx b/qemu-options.hx
> index 1cf9aac..033e311 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -206,10 +206,14 @@ For example:
>  -numa cpu,node-id=0,socket-id=0 -numa cpu,node-id=1,socket-id=1
>  @end example
>  
> -@samp{mem} assigns a given RAM amount to a node. @samp{memdev}
> -assigns RAM from a given memory backend device to a node. If
> -@samp{mem} and @samp{memdev} are omitted in all nodes, RAM is
> -split equally between them.
> +@samp{memdev} assigns RAM from a given memory backend device to a node.
> +
> +Legacy options/behaviour: @samp{mem} assigns a given RAM amount to a node.
> +If @samp{mem} and @samp{memdev} are omitted in all nodes, RAM is split equally
> +between them. Option @samp{mem} and default RAM splitting are obsolete as they
> +do not provide means to manage RAM on host side and only allow QEMU to fake
> +NUMA support which in practice could degrade VM performance.
> +It's advised to always explicitly configure NUMA RAM using @samp{memdev} option.
>  
>  @samp{mem} and @samp{memdev} are mutually exclusive. Furthermore,
>  if one node uses @samp{memdev}, all of them have to use it.
> -- 
> 2.7.4
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] numa: warn if numa 'mem' option or default RAM splitting between nodes is used.

[Igor Mammedov]

On Wed, 6 Mar 2019 16:39:38 +0000
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Wed, Mar 06, 2019 at 05:30:25PM +0100, Igor Mammedov wrote:
> > Ammend -numa option docs and print warnings if 'mem' option or default RAM
> > splitting between nodes is used. It's intended to discourage users from using
> > configuration that allows only to fake NUMA on guest side while leading
> > to reduced performance of the guest due to inability to properly configure
> > VM's RAM on the host.
> > 
> > In NUMA case, it's recommended to always explicitly configure guest RAM
> > using -numa node,memdev={backend-id} option.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  numa.c          |  5 +++++
> >  qemu-options.hx | 12 ++++++++----
> >  2 files changed, 13 insertions(+), 4 deletions(-)
> > 
> > diff --git a/numa.c b/numa.c
> > index 3875e1e..c6c2a6f 100644
> > --- a/numa.c
> > +++ b/numa.c
> > @@ -121,6 +121,8 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
> >  
> >      if (node->has_mem) {
> >          numa_info[nodenr].node_mem = node->mem;
> > +        warn_report("Parameter -numa node,mem is obsolete,"
> > +                    " use -numa node,memdev instead");
> 
> I don't think we should do this. Libvirt isn't going to stop using this
> option in the near term. When users see warnings like this in logs
well when it was the only option available libvirt had no other choice,
but since memdev became available libvirt should try to use it whenever
possible.

> they'll often file bugs reports thinking something is broken which is
> not the case here. 
It's the exact purpose of the warning, to force user asking questions
and fix configuration, since he/she obviously not getting NUMA benefits
and/or performance-wise

I have to disagree here, I don't like ducking our head and ignoring broken
configuration when there is working alternative. As end user I'd hate that
problem was hidden from me.


I'll respin v2 after fixing mistakes Eric pointed out.

> 
> >      }
> >      if (node->has_memdev) {
> >          Object *o;
> > @@ -407,6 +409,9 @@ void numa_complete_configuration(MachineState *ms)
> >          if (i == nb_numa_nodes) {
> >              assert(mc->numa_auto_assign_ram);
> >              mc->numa_auto_assign_ram(mc, numa_info, nb_numa_nodes, ram_size);
> > +            warn_report("Default splitting of RAM between nodes is obsolete,"
> > +                        " Use '-numa node,memdev' to explictly define RAM"
> > +                        " allocation per node");
> 
> I think this one is ok as I don't believe libvirt triggers this code path
> 
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 1cf9aac..033e311 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -206,10 +206,14 @@ For example:
> >  -numa cpu,node-id=0,socket-id=0 -numa cpu,node-id=1,socket-id=1
> >  @end example
> >  
> > -@samp{mem} assigns a given RAM amount to a node. @samp{memdev}
> > -assigns RAM from a given memory backend device to a node. If
> > -@samp{mem} and @samp{memdev} are omitted in all nodes, RAM is
> > -split equally between them.
> > +@samp{memdev} assigns RAM from a given memory backend device to a node.
> > +
> > +Legacy options/behaviour: @samp{mem} assigns a given RAM amount to a node.
> > +If @samp{mem} and @samp{memdev} are omitted in all nodes, RAM is split equally
> > +between them. Option @samp{mem} and default RAM splitting are obsolete as they
> > +do not provide means to manage RAM on host side and only allow QEMU to fake
> > +NUMA support which in practice could degrade VM performance.
> > +It's advised to always explicitly configure NUMA RAM using @samp{memdev} option.
> >  
> >  @samp{mem} and @samp{memdev} are mutually exclusive. Furthermore,
> >  if one node uses @samp{memdev}, all of them have to use it.
> > -- 
> > 2.7.4
> > 
> 
> Regards,
> Daniel

Re: [Qemu-devel] [PATCH] numa: warn if numa 'mem' option or default RAM splitting between nodes is used.

[Daniel P. Berrangé]

On Wed, Mar 06, 2019 at 05:58:35PM +0100, Igor Mammedov wrote:
> On Wed, 6 Mar 2019 16:39:38 +0000
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> 
> > On Wed, Mar 06, 2019 at 05:30:25PM +0100, Igor Mammedov wrote:
> > > Ammend -numa option docs and print warnings if 'mem' option or default RAM
> > > splitting between nodes is used. It's intended to discourage users from using
> > > configuration that allows only to fake NUMA on guest side while leading
> > > to reduced performance of the guest due to inability to properly configure
> > > VM's RAM on the host.
> > > 
> > > In NUMA case, it's recommended to always explicitly configure guest RAM
> > > using -numa node,memdev={backend-id} option.
> > > 
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > >  numa.c          |  5 +++++
> > >  qemu-options.hx | 12 ++++++++----
> > >  2 files changed, 13 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/numa.c b/numa.c
> > > index 3875e1e..c6c2a6f 100644
> > > --- a/numa.c
> > > +++ b/numa.c
> > > @@ -121,6 +121,8 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
> > >  
> > >      if (node->has_mem) {
> > >          numa_info[nodenr].node_mem = node->mem;
> > > +        warn_report("Parameter -numa node,mem is obsolete,"
> > > +                    " use -numa node,memdev instead");
> > 
> > I don't think we should do this. Libvirt isn't going to stop using this
> > option in the near term. When users see warnings like this in logs
> well when it was the only option available libvirt had no other choice,
> but since memdev became available libvirt should try to use it whenever
> possible.

As we previously discussed, it is not possible for libvirt to use it
in all cases.

> 
> > they'll often file bugs reports thinking something is broken which is
> > not the case here. 
> It's the exact purpose of the warning, to force user asking questions
> and fix configuration, since he/she obviously not getting NUMA benefits
> and/or performance-wise

That's only useful if it is possible to do something about the problem.
Libvirt wants to use the new option but it can't due to the live migration
problems. So this simply leads to bug reports that will end up marked
as CANTFIX.

I don't believe libvirt actually  suffers from the performance problem
you describe wrt lack of pinning.   When we attempt to pin guest NUMA
nodes to host NUMA nodes, libvirt *will* use "memdev". IIUC, we
use "mem" in the case where there /no/ requested pinning of guest
NUMA nodes, and so we're not suffering from the limitations of "mem"
in that case.

> I have to disagree here, I don't like ducking our head and ignoring broken
> configuration when there is working alternative. As end user I'd hate that
> problem was hidden from me.



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] numa: warn if numa 'mem' option or default RAM splitting between nodes is used.

[Igor Mammedov]

On Wed, 6 Mar 2019 17:10:37 +0000
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Wed, Mar 06, 2019 at 05:58:35PM +0100, Igor Mammedov wrote:
> > On Wed, 6 Mar 2019 16:39:38 +0000
> > Daniel P. Berrangé <berrange@redhat.com> wrote:
> > 
> > > On Wed, Mar 06, 2019 at 05:30:25PM +0100, Igor Mammedov wrote:
> > > > Ammend -numa option docs and print warnings if 'mem' option or default RAM
> > > > splitting between nodes is used. It's intended to discourage users from using
> > > > configuration that allows only to fake NUMA on guest side while leading
> > > > to reduced performance of the guest due to inability to properly configure
> > > > VM's RAM on the host.
> > > > 
> > > > In NUMA case, it's recommended to always explicitly configure guest RAM
> > > > using -numa node,memdev={backend-id} option.
> > > > 
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > ---
> > > >  numa.c          |  5 +++++
> > > >  qemu-options.hx | 12 ++++++++----
> > > >  2 files changed, 13 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/numa.c b/numa.c
> > > > index 3875e1e..c6c2a6f 100644
> > > > --- a/numa.c
> > > > +++ b/numa.c
> > > > @@ -121,6 +121,8 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
> > > >  
> > > >      if (node->has_mem) {
> > > >          numa_info[nodenr].node_mem = node->mem;
> > > > +        warn_report("Parameter -numa node,mem is obsolete,"
> > > > +                    " use -numa node,memdev instead");
> > > 
> > > I don't think we should do this. Libvirt isn't going to stop using this
> > > option in the near term. When users see warnings like this in logs
> > well when it was the only option available libvirt had no other choice,
> > but since memdev became available libvirt should try to use it whenever
> > possible.
> 
> As we previously discussed, it is not possible for libvirt to use it
> in all cases.
> 
> > 
> > > they'll often file bugs reports thinking something is broken which is
> > > not the case here. 
> > It's the exact purpose of the warning, to force user asking questions
> > and fix configuration, since he/she obviously not getting NUMA benefits
> > and/or performance-wise
> 
> That's only useful if it is possible to do something about the problem.
> Libvirt wants to use the new option but it can't due to the live migration
> problems. So this simply leads to bug reports that will end up marked
> as CANTFIX.
The problem could be solved by user though, by reconfiguring and restarting
domain since it's impossible to (at least as it stands now wrt migration).

> I don't believe libvirt actually  suffers from the performance problem
> you describe wrt lack of pinning.   When we attempt to pin guest NUMA
> nodes to host NUMA nodes, libvirt *will* use "memdev". IIUC, we
> use "mem" in the case where there /no/ requested pinning of guest
> NUMA nodes, and so we're not suffering from the limitations of "mem"
> in that case.
What would be the use-case for not pinning numa nodes?
If user isn't asking for pinning, VM would run with degraded performance and
it would be better of being non-numa.

Even if user doesn't ask for pinning, non-pinned memdev (for new VMs where
available) could be used as well. Users of 'mem' with new QEMU will see the warning
and probably think about if they are configured VM correctly.

Desire to deprecate 'mem' at least for new machines is to be able to prevent user
from creating broken configs and manage all RAM in the same manner (frontend/backend).
(I can't do it for old machine types but for new it is possible).


> > I have to disagree here, I don't like ducking our head and ignoring broken
> > configuration when there is working alternative. As end user I'd hate that
> > problem was hidden from me.
> 
> 
> 
> Regards,
> Daniel

Re: [Qemu-devel] [PATCH] numa: warn if numa 'mem' option or default RAM splitting between nodes is used.

[Daniel P. Berrangé]

On Wed, Mar 06, 2019 at 07:48:22PM +0100, Igor Mammedov wrote:
> On Wed, 6 Mar 2019 17:10:37 +0000
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> 
> > On Wed, Mar 06, 2019 at 05:58:35PM +0100, Igor Mammedov wrote:
> > > On Wed, 6 Mar 2019 16:39:38 +0000
> > > Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > 
> > > > On Wed, Mar 06, 2019 at 05:30:25PM +0100, Igor Mammedov wrote:
> > > > > Ammend -numa option docs and print warnings if 'mem' option or default RAM
> > > > > splitting between nodes is used. It's intended to discourage users from using
> > > > > configuration that allows only to fake NUMA on guest side while leading
> > > > > to reduced performance of the guest due to inability to properly configure
> > > > > VM's RAM on the host.
> > > > > 
> > > > > In NUMA case, it's recommended to always explicitly configure guest RAM
> > > > > using -numa node,memdev={backend-id} option.
> > > > > 
> > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > > ---
> > > > >  numa.c          |  5 +++++
> > > > >  qemu-options.hx | 12 ++++++++----
> > > > >  2 files changed, 13 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/numa.c b/numa.c
> > > > > index 3875e1e..c6c2a6f 100644
> > > > > --- a/numa.c
> > > > > +++ b/numa.c
> > > > > @@ -121,6 +121,8 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
> > > > >  
> > > > >      if (node->has_mem) {
> > > > >          numa_info[nodenr].node_mem = node->mem;
> > > > > +        warn_report("Parameter -numa node,mem is obsolete,"
> > > > > +                    " use -numa node,memdev instead");
> > > > 
> > > > I don't think we should do this. Libvirt isn't going to stop using this
> > > > option in the near term. When users see warnings like this in logs
> > > well when it was the only option available libvirt had no other choice,
> > > but since memdev became available libvirt should try to use it whenever
> > > possible.
> > 
> > As we previously discussed, it is not possible for libvirt to use it
> > in all cases.
> > 
> > > 
> > > > they'll often file bugs reports thinking something is broken which is
> > > > not the case here. 
> > > It's the exact purpose of the warning, to force user asking questions
> > > and fix configuration, since he/she obviously not getting NUMA benefits
> > > and/or performance-wise
> > 
> > That's only useful if it is possible to do something about the problem.
> > Libvirt wants to use the new option but it can't due to the live migration
> > problems. So this simply leads to bug reports that will end up marked
> > as CANTFIX.
> The problem could be solved by user though, by reconfiguring and restarting
> domain since it's impossible to (at least as it stands now wrt migration).
> 
> > I don't believe libvirt actually  suffers from the performance problem
> > you describe wrt lack of pinning.   When we attempt to pin guest NUMA
> > nodes to host NUMA nodes, libvirt *will* use "memdev". IIUC, we
> > use "mem" in the case where there /no/ requested pinning of guest
> > NUMA nodes, and so we're not suffering from the limitations of "mem"
> > in that case.
> What would be the use-case for not pinning numa nodes?
> If user isn't asking for pinning, VM would run with degraded performance and
> it would be better of being non-numa.

The guest could have been originally booted on a host which has 2 NUMA
nodes and have been migrated to a host with 1 NUMA node, in which case
pinnning is not relevant.

For CI purposes too it is reasonable to create guests with NUMA configurations
that bear no resemblance to the host NUMA configuration. This allows for testing
the operation of guest applications. This is something that is relevant to
OpenStack for testnig Nova's handling of NUMA placement logic, since almost
all their testing is in VMs not bare metal.

Not pinning isn't common, but it is reasonable to do it.

> Even if user doesn't ask for pinning, non-pinned memdev (for new VMs where
> available) could be used as well. Users of 'mem' with new QEMU will see the warning
> and probably think about if they are configured VM correctly.

We can't change to memdev due to migration problems. Since there is no
functional problem with using 'mem' in this scenario there's no pressing
reason to stop using 'mem'.

> Desire to deprecate 'mem' at least for new machines is to be able to prevent user
> from creating broken configs and manage all RAM in the same manner (frontend/backend).
> (I can't do it for old machine types but for new it is possible).

As before libvirt can't make its CLI config dependant on machine type
version as that breaks live migration to old libvirt versions.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] numa: warn if numa 'mem' option or default RAM splitting between nodes is used.

[Igor Mammedov]

On Thu, 7 Mar 2019 10:04:56 +0000
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Wed, Mar 06, 2019 at 07:48:22PM +0100, Igor Mammedov wrote:
> > On Wed, 6 Mar 2019 17:10:37 +0000
> > Daniel P. Berrangé <berrange@redhat.com> wrote:
> >   
> > > On Wed, Mar 06, 2019 at 05:58:35PM +0100, Igor Mammedov wrote:  
> > > > On Wed, 6 Mar 2019 16:39:38 +0000
> > > > Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > >   
> > > > > On Wed, Mar 06, 2019 at 05:30:25PM +0100, Igor Mammedov wrote:  
> > > > > > Ammend -numa option docs and print warnings if 'mem' option or default RAM
> > > > > > splitting between nodes is used. It's intended to discourage users from using
> > > > > > configuration that allows only to fake NUMA on guest side while leading
> > > > > > to reduced performance of the guest due to inability to properly configure
> > > > > > VM's RAM on the host.
> > > > > > 
> > > > > > In NUMA case, it's recommended to always explicitly configure guest RAM
> > > > > > using -numa node,memdev={backend-id} option.
> > > > > > 
> > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > > > ---
> > > > > >  numa.c          |  5 +++++
> > > > > >  qemu-options.hx | 12 ++++++++----
> > > > > >  2 files changed, 13 insertions(+), 4 deletions(-)
> > > > > > 
> > > > > > diff --git a/numa.c b/numa.c
> > > > > > index 3875e1e..c6c2a6f 100644
> > > > > > --- a/numa.c
> > > > > > +++ b/numa.c
> > > > > > @@ -121,6 +121,8 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
> > > > > >  
> > > > > >      if (node->has_mem) {
> > > > > >          numa_info[nodenr].node_mem = node->mem;
> > > > > > +        warn_report("Parameter -numa node,mem is obsolete,"
> > > > > > +                    " use -numa node,memdev instead");  
> > > > > 
> > > > > I don't think we should do this. Libvirt isn't going to stop using this
> > > > > option in the near term. When users see warnings like this in logs  
> > > > well when it was the only option available libvirt had no other choice,
> > > > but since memdev became available libvirt should try to use it whenever
> > > > possible.  
> > > 
> > > As we previously discussed, it is not possible for libvirt to use it
> > > in all cases.
> > >   
> > > >   
> > > > > they'll often file bugs reports thinking something is broken which is
> > > > > not the case here.   
> > > > It's the exact purpose of the warning, to force user asking questions
> > > > and fix configuration, since he/she obviously not getting NUMA benefits
> > > > and/or performance-wise  
> > > 
> > > That's only useful if it is possible to do something about the problem.
> > > Libvirt wants to use the new option but it can't due to the live migration
> > > problems. So this simply leads to bug reports that will end up marked
> > > as CANTFIX.  
> > The problem could be solved by user though, by reconfiguring and restarting
> > domain since it's impossible to (at least as it stands now wrt migration).
> >   
> > > I don't believe libvirt actually  suffers from the performance problem
> > > you describe wrt lack of pinning.   When we attempt to pin guest NUMA
> > > nodes to host NUMA nodes, libvirt *will* use "memdev". IIUC, we
> > > use "mem" in the case where there /no/ requested pinning of guest
> > > NUMA nodes, and so we're not suffering from the limitations of "mem"
> > > in that case.  
> > What would be the use-case for not pinning numa nodes?
> > If user isn't asking for pinning, VM would run with degraded performance and
> > it would be better of being non-numa.  
> 
> The guest could have been originally booted on a host which has 2 NUMA
> nodes and have been migrated to a host with 1 NUMA node, in which case
> pinnning is not relevant.
> 
> For CI purposes too it is reasonable to create guests with NUMA configurations
> that bear no resemblance to the host NUMA configuration. This allows for testing
> the operation of guest applications. This is something that is relevant to
> OpenStack for testnig Nova's handling of NUMA placement logic, since almost
> all their testing is in VMs not bare metal.
>
> Not pinning isn't common, but it is reasonable to do it.

 
 
> > Even if user doesn't ask for pinning, non-pinned memdev (for new VMs where
> > available) could be used as well. Users of 'mem' with new QEMU will see the warning
> > and probably think about if they are configured VM correctly.  
> 
> We can't change to memdev due to migration problems. Since there is no
> functional problem with using 'mem' in this scenario there's no pressing
> reason to stop using 'mem'.
The problem with it is that it blocks fixing bug in QEMU for the multi node case
 '-numa node,mem + -mem-path + -mem-prealloc + -object "memdev",policy=bind'
and it also gets in the way of unifying RAM handling using device-memory
for initial RAM due to the same migration issue since RAM layout in migration
stream is different.

> > Desire to deprecate 'mem' at least for new machines is to be able to prevent user
> > from creating broken configs and manage all RAM in the same manner (frontend/backend).
> > (I can't do it for old machine types but for new it is possible).  
> 
> As before libvirt can't make its CLI config dependant on machine type
> version as that breaks live migration to old libvirt versions.
> 
> Regards,
> Daniel