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

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

[Igor Mammedov]

Amend -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..42838f9 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 explicitly define RAM"
+                        " allocation per node");
         }
 
         numa_total = 0;
diff --git a/qemu-options.hx b/qemu-options.hx
index 1cf9aac..61035cb 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 the 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 by using the @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 v2] numa: warn if numa 'mem' option or default RAM splitting between nodes is used.

[Daniel P. Berrangé]

On Wed, Mar 06, 2019 at 06:33:25PM +0100, Igor Mammedov wrote:
> Amend -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..42838f9 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");

My comments from v1 still apply. We must not do this as long as
libvirt has no choice but to continue using this feature.

>      }
>      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 explicitly define RAM"
> +                        " allocation per node");
>          }
>  
>          numa_total = 0;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 1cf9aac..61035cb 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 the 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 by using the @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 v2] numa: warn if numa 'mem' option or default RAM splitting between nodes is used.

[Igor Mammedov]

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

> On Wed, Mar 06, 2019 at 06:33:25PM +0100, Igor Mammedov wrote:
> > Amend -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..42838f9 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");
> 
> My comments from v1 still apply. We must not do this as long as
> libvirt has no choice but to continue using this feature.
It has a choice to use 'memdev' whenever creating a new VM and continue using 'mem' with exiting VMs.

> 
> >      }
> >      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 explicitly define RAM"
> > +                        " allocation per node");
> >          }
> >  
> >          numa_total = 0;
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 1cf9aac..61035cb 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 the 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 by using the @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 v2] numa: warn if numa 'mem' option or default RAM splitting between nodes is used.

[Daniel P. Berrangé]

On Wed, Mar 06, 2019 at 07:54:17PM +0100, Igor Mammedov wrote:
> On Wed, 6 Mar 2019 18:16:08 +0000
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> 
> > On Wed, Mar 06, 2019 at 06:33:25PM +0100, Igor Mammedov wrote:
> > > Amend -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..42838f9 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");
> > 
> > My comments from v1 still apply. We must not do this as long as
> > libvirt has no choice but to continue using this feature.
> It has a choice to use 'memdev' whenever creating a new VM and continue
> using 'mem' with exiting VMs.

Unfortunately we don't have such a choice. Libvirt has no concept of the
distinction between an 'existing' and 'new' VM. It just receives an XML
file from the mgmt application and with transient guests, we have no
persistent configuration record of the VM. So we've no way of knowing
whether this VM was previously running on this same host, or another
host, or is completely new.


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