From f11abb8b74b6d1ed446397fe441cb94e239b5ab6 Mon Sep 17 00:00:00 2001 From: Homero Pawlowski Date: Wed, 2 Aug 2017 01:07:23 -0400 Subject: [PATCH 1/8] refactored and fixed numa_nodes domain specific option --- README.md | 10 ++++++++- lib/vagrant-libvirt/action/start_domain.rb | 17 +++++++-------- lib/vagrant-libvirt/config.rb | 22 ++++---------------- lib/vagrant-libvirt/templates/domain.xml.erb | 15 +++++++------ 4 files changed, 27 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 12e8d40..fc9adee 100644 --- a/README.md +++ b/README.md @@ -300,7 +300,15 @@ end * `cpu_fallback` - Whether to allow libvirt to fall back to a CPU model close to the specified model if features in the guest CPU are not supported on the host. Defaults to 'allow' if not set. Allowed values: `allow`, `forbid`. -* `numa_nodes` - Number of NUMA nodes on guest. Must be a factor of `cpu`. +* `numa_nodes` - Specify an array of NUMA nodes for the guest. The syntax is similar to what would be set in the domain XML. `memory` must be in MB. Symmetrical and asymmetrical topologies are supported but ake sure your total count of defined CPUs adds up to v.cpus. + + The sum of all the memory defined here will act as your total memory for your guest VM. **This sum will override what is set in `v.memory`** + ``` + v.numa_nodes = [ + {:id => 0, :cpus => "0-1", :memory => "1024"}, + {:id => 1, :cpus => "2-3", :memory => "4096"} + ] + ``` * `loader` - Sets path to custom UEFI loader. * `volume_cache` - Controls the cache mechanism. Possible values are "default", "none", "writethrough", "writeback", "directsync" and "unsafe". [See diff --git a/lib/vagrant-libvirt/action/start_domain.rb b/lib/vagrant-libvirt/action/start_domain.rb index 0a75bd2..09495d2 100644 --- a/lib/vagrant-libvirt/action/start_domain.rb +++ b/lib/vagrant-libvirt/action/start_domain.rb @@ -23,9 +23,13 @@ module VagrantPlugins libvirt_domain = env[:machine].provider.driver.connection.client.lookup_domain_by_uuid(env[:machine].id) - if config.memory.to_i * 1024 != libvirt_domain.max_memory - libvirt_domain.max_memory = config.memory.to_i * 1024 - libvirt_domain.memory = libvirt_domain.max_memory + # libvirt API doesn't support modifying memory on NUMA enabled CPUs + # http://libvirt.org/git/?p=libvirt.git;a=commit;h=d174394105cf00ed266bf729ddf461c21637c736 + if config.numa_nodes == nil + if config.memory.to_i * 1024 != libvirt_domain.max_memory + libvirt_domain.max_memory = config.memory.to_i * 1024 + libvirt_domain.memory = libvirt_domain.max_memory + end end begin # XML definition manipulation @@ -124,13 +128,6 @@ module VagrantPlugins cpu.delete_element(svm_feature) end end - else - unless cpu.elements.to_a.empty? - descr_changed = true - cpu.elements.each do |elem| - cpu.delete_element(elem) - end - end end # Graphics diff --git a/lib/vagrant-libvirt/config.rb b/lib/vagrant-libvirt/config.rb index a068cfa..b0a1ccd 100644 --- a/lib/vagrant-libvirt/config.rb +++ b/lib/vagrant-libvirt/config.rb @@ -279,26 +279,12 @@ module VagrantPlugins end def _generate_numa - raise 'NUMA nodes must be a factor of CPUs' if @cpus % @numa_nodes != 0 + # todo: compare count of NUMA defined CPUs and throw warning + # if config.cpu != NUMA.cpu - if @memory % @numa_nodes != 0 - raise 'NUMA nodes must be a factor of memory' - end + @numa_nodes.collect { |x| x[:memory] = x[:memory].to_i * 1024 } - numa = [] - - (1..@numa_nodes).each do |node| - numa_cpu_start = (@cpus / @numa_nodes) * (node - 1) - numa_cpu_end = (@cpus / @numa_nodes) * node - 1 - numa_cpu = Array(numa_cpu_start..numa_cpu_end).join(',') - numa_mem = @memory / @numa_nodes - - numa.push(id: node, - cpu: numa_cpu, - mem: numa_mem) - end - - @numa_nodes = numa + @numa_nodes end def cpu_feature(options = {}) diff --git a/lib/vagrant-libvirt/templates/domain.xml.erb b/lib/vagrant-libvirt/templates/domain.xml.erb index b216300..6e79ecc 100644 --- a/lib/vagrant-libvirt/templates/domain.xml.erb +++ b/lib/vagrant-libvirt/templates/domain.xml.erb @@ -15,14 +15,13 @@ <% @cpu_features.each do |cpu_feature| %> <% end %> - <% else %> - <% if @numa_nodes %> - - <% @numa_nodes.each do |node| %> - - <% end %> - - <% end %> + <% end %> + <% if @numa_nodes %> + + <% @numa_nodes.each do |node| %> + + <% end %> + <% end %> From cd4d9739b478ee4cfc2453477658fe909732a30b Mon Sep 17 00:00:00 2001 From: Homero Pawlowski Date: Wed, 2 Aug 2017 01:09:59 -0400 Subject: [PATCH 2/8] fixed README typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index fc9adee..d86241f 100644 --- a/README.md +++ b/README.md @@ -300,7 +300,7 @@ end * `cpu_fallback` - Whether to allow libvirt to fall back to a CPU model close to the specified model if features in the guest CPU are not supported on the host. Defaults to 'allow' if not set. Allowed values: `allow`, `forbid`. -* `numa_nodes` - Specify an array of NUMA nodes for the guest. The syntax is similar to what would be set in the domain XML. `memory` must be in MB. Symmetrical and asymmetrical topologies are supported but ake sure your total count of defined CPUs adds up to v.cpus. +* `numa_nodes` - Specify an array of NUMA nodes for the guest. The syntax is similar to what would be set in the domain XML. `memory` must be in MB. Symmetrical and asymmetrical topologies are supported but make sure your total count of defined CPUs adds up to v.cpus. The sum of all the memory defined here will act as your total memory for your guest VM. **This sum will override what is set in `v.memory`** ``` From 4f5351ab1d2b1d731edd1bb1944e3f813a0a53fe Mon Sep 17 00:00:00 2001 From: Homero Pawlowski Date: Wed, 2 Aug 2017 01:11:13 -0400 Subject: [PATCH 3/8] README change --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d86241f..8ad0596 100644 --- a/README.md +++ b/README.md @@ -300,7 +300,7 @@ end * `cpu_fallback` - Whether to allow libvirt to fall back to a CPU model close to the specified model if features in the guest CPU are not supported on the host. Defaults to 'allow' if not set. Allowed values: `allow`, `forbid`. -* `numa_nodes` - Specify an array of NUMA nodes for the guest. The syntax is similar to what would be set in the domain XML. `memory` must be in MB. Symmetrical and asymmetrical topologies are supported but make sure your total count of defined CPUs adds up to v.cpus. +* `numa_nodes` - Specify an array of NUMA nodes for the guest. The syntax is similar to what would be set in the domain XML. `memory` must be in MB. Symmetrical and asymmetrical topologies are supported but make sure your total count of defined CPUs adds up to `v.cpus`. The sum of all the memory defined here will act as your total memory for your guest VM. **This sum will override what is set in `v.memory`** ``` From 45e41e2e1086f318b7e6328ed60e5ef09ef74a90 Mon Sep 17 00:00:00 2001 From: Homero Pawlowski Date: Wed, 2 Aug 2017 01:16:40 -0400 Subject: [PATCH 4/8] README change --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 8ad0596..0801136 100644 --- a/README.md +++ b/README.md @@ -304,6 +304,7 @@ end The sum of all the memory defined here will act as your total memory for your guest VM. **This sum will override what is set in `v.memory`** ``` + v.cpus = 4 v.numa_nodes = [ {:id => 0, :cpus => "0-1", :memory => "1024"}, {:id => 1, :cpus => "2-3", :memory => "4096"} From 10ed9fbc7ea9054043e15591d01ab2424c6b19de Mon Sep 17 00:00:00 2001 From: Homero Pawlowski Date: Fri, 11 Aug 2017 16:27:28 -0400 Subject: [PATCH 5/8] added back block to remove nested cpu elements if host-passthrough is set, UNLESS numa_nodes is set --- lib/vagrant-libvirt/action/start_domain.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/vagrant-libvirt/action/start_domain.rb b/lib/vagrant-libvirt/action/start_domain.rb index 09495d2..7461905 100644 --- a/lib/vagrant-libvirt/action/start_domain.rb +++ b/lib/vagrant-libvirt/action/start_domain.rb @@ -128,6 +128,13 @@ module VagrantPlugins cpu.delete_element(svm_feature) end end + elsif config.numa_nodes == nil + unless cpu.elements.to_a.empty? + descr_changed = true + cpu.elements.each do |elem| + cpu.delete_element(elem) + end + end end # Graphics From 8bf16ba34f5fc32e6e2467ce9c7ae78d22597149 Mon Sep 17 00:00:00 2001 From: Homero Pawlowski Date: Tue, 15 Aug 2017 17:02:06 -0400 Subject: [PATCH 6/8] added some validation around @numa_nodes[:cpus] values --- lib/vagrant-libvirt/config.rb | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/vagrant-libvirt/config.rb b/lib/vagrant-libvirt/config.rb index b0a1ccd..efc27be 100644 --- a/lib/vagrant-libvirt/config.rb +++ b/lib/vagrant-libvirt/config.rb @@ -279,8 +279,22 @@ module VagrantPlugins end def _generate_numa - # todo: compare count of NUMA defined CPUs and throw warning - # if config.cpu != NUMA.cpu + # Perform some validation of cpu values + @numa_nodes.each { |x| + unless x[:cpus] =~ /^\d+-\d+$/ + raise 'numa_nodes[:cpus] must be in format "integer-integer"' + end + } + + # Grab the value of the last @numa_nodes[:cpus] and verify @cpus matches + # Note: [:cpus] is zero based and @cpus is not, so we need to +1 + last_cpu = @numa_nodes.last[:cpus] + last_cpu = last_cpu.scan(/\d+$/)[0] + last_cpu = last_cpu.to_i + 1 + + if @cpus != last_cpu.to_i + raise 'The total number of numa_nodes[:cpus] must equal config.cpus' + end @numa_nodes.collect { |x| x[:memory] = x[:memory].to_i * 1024 } From 26b3e3c8fe3986062304c1d96fd08d7d8f467f57 Mon Sep 17 00:00:00 2001 From: Homero Pawlowski Date: Wed, 16 Aug 2017 11:50:29 -0400 Subject: [PATCH 7/8] automatically insert numa_cpus[:id] based on array index, and remove user's ability to edit --- README.md | 4 ++-- lib/vagrant-libvirt/templates/domain.xml.erb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 0801136..472ca1a 100644 --- a/README.md +++ b/README.md @@ -306,8 +306,8 @@ end ``` v.cpus = 4 v.numa_nodes = [ - {:id => 0, :cpus => "0-1", :memory => "1024"}, - {:id => 1, :cpus => "2-3", :memory => "4096"} + {:cpus => "0-1", :memory => "1024"}, + {:cpus => "2-3", :memory => "4096"} ] ``` * `loader` - Sets path to custom UEFI loader. diff --git a/lib/vagrant-libvirt/templates/domain.xml.erb b/lib/vagrant-libvirt/templates/domain.xml.erb index 6e79ecc..7fd991e 100644 --- a/lib/vagrant-libvirt/templates/domain.xml.erb +++ b/lib/vagrant-libvirt/templates/domain.xml.erb @@ -18,8 +18,8 @@ <% end %> <% if @numa_nodes %> - <% @numa_nodes.each do |node| %> - + <% @numa_nodes.each_with_index do |node, index| %> + <% end %> <% end %> From ebd2cb48d0b03e2ddde909757157c055209f5802 Mon Sep 17 00:00:00 2001 From: Homero Pawlowski Date: Thu, 17 Aug 2017 15:43:46 -0400 Subject: [PATCH 8/8] refactored to only iterate numa_nodes array once --- lib/vagrant-libvirt/config.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/vagrant-libvirt/config.rb b/lib/vagrant-libvirt/config.rb index efc27be..d75a08c 100644 --- a/lib/vagrant-libvirt/config.rb +++ b/lib/vagrant-libvirt/config.rb @@ -279,13 +279,16 @@ module VagrantPlugins end def _generate_numa - # Perform some validation of cpu values - @numa_nodes.each { |x| + @numa_nodes.collect { |x| + # Perform some validation of cpu values unless x[:cpus] =~ /^\d+-\d+$/ raise 'numa_nodes[:cpus] must be in format "integer-integer"' end + + # Convert to KiB + x[:memory] = x[:memory].to_i * 1024 } - + # Grab the value of the last @numa_nodes[:cpus] and verify @cpus matches # Note: [:cpus] is zero based and @cpus is not, so we need to +1 last_cpu = @numa_nodes.last[:cpus] @@ -296,8 +299,6 @@ module VagrantPlugins raise 'The total number of numa_nodes[:cpus] must equal config.cpus' end - @numa_nodes.collect { |x| x[:memory] = x[:memory].to_i * 1024 } - @numa_nodes end