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

On Tue, Mar 19, 2019 at 02:08:01PM +0100, Igor Mammedov wrote: > On Thu, 7 Mar 2019 10:07:05 +0000 > Daniel P. Berrangé <berrange(a)redhat.com&gt; wrote: > > > 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(a)redhat.com&gt; 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(a)redhat.com&gt; > > > > > --- > > > > > 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. > In case of transient VM, libvirt might be able to use machine version > as deciding which option to use (memdev is around more than 4 years since 2.1) > (or QEMU could provide introspection into what machine version (not)supports, > like it was discussed before) > > As discussed elsewhere (v1 tread|IRC), there are users (mainly CI) for which > fake NUMA is sufficient and they do not ask for explicit pinning, so libvirt > defaults to legacy -numa node,mem option. > Those users do not care no aware that they should use memdev instead > (I'm not sure if they are able to ask libvirt for non pinned numa memory > which results in memdev being used). > This patch doesn't obsolete anything yet, it serves purpose to inform users > that they are using legacy option and advises replacement option > so that users would know to what they should adapt to. > > Once we deprecate and then remove 'mem' for new machines only (while keeping > 'mem' working on old machine versions). The new nor old libvirt won't be able > to start new machine type with 'mem' option and have to use memdev variant, > so we don't have migration issues with new machines and old ones continue > working with 'mem'. I'm not seeing what has changed which would enable us to deprecate something only for new machines. That's not possible from libvirt's POV as old libvirt will support new machines & thus we have to continue using "mem" for all machines in the scenarios where we currently use it.