[PATCH/foreman 1/1] fixes #1667 - Wrong parameter naming caused Libvirt to ignore memory size

Olivier_Favre · June 11, 2012, 9:21am

Also divide memory size by 1024 as Libvirt wants KiB, not bytes.

Signed-off-by: Olivier Favre <olivier@yakaz.com>

···

--- .../compute_resources_vms/form/_libvirt.html.erb | 2 +- lib/foreman/model/libvirt.rb | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/app/views/compute_resources_vms/form/_libvirt.html.erb b/app/views/compute_resources_vms/form/_libvirt.html.erb
index 94378f2…3bf833a 100644
— a/app/views/compute_resources_vms/form/_libvirt.html.erb
+++ b/app/views/compute_resources_vms/form/_libvirt.html.erb
@@ -1,7 +1,7 @@
<% new = f.object && f.object.new? -%>
<%= text_f f, :name, :disabled => !new if controller_name != “hosts” %>
<%= selectable_f f, :cpus, 1…compute_resource.max_cpu_count, { }, :class => “input-mini”,:disabled => !new %>
-<%= selectable_f f, :memory, memory_options(compute_resource.max_memory), { }, :class => “span2”, :disabled => !new %>
+<%= selectable_f f, :memory_size, memory_options(compute_resource.max_memory), { }, :class => “span2”, :disabled => !new %>

diff --git a/lib/foreman/model/libvirt.rb b/lib/foreman/model/libvirt.rb index bd21972..79bb8a0 100644 --- a/lib/foreman/model/libvirt.rb +++ b/lib/foreman/model/libvirt.rb @@ -64,6 +64,7 @@ module Foreman::Model end

 def new_vm attr={ }

 attr[:memory_size] = attr[:memory_size].to_i / 1024 if attr.member? :memory_size
 opts = vm_instance_defaults.merge(attr.to_hash).symbolize_keys

 # convert rails nested_attributes into a plain hash

@@ -123,11 +124,11 @@ module Foreman::Model

 def vm_instance_defaults
   {

```
   :memory     => 768*1024*1024,
```
```
   :boot_order => %w[network hd],
```
```
   :nics       => [new_nic],
```
```
   :volumes    => [new_volume],
```

   :display    => { :type => 'vnc', :listen => '0', :password => random_password, :port => '-1' }

```
   :memory_size => 768*1024*1024,
```
```
   :boot_order  => %w[network hd],
```
```
   :nics        => [new_nic],
```
```
   :volumes     => [new_volume],
```

   :display     => { :type => 'vnc', :listen => '0', :password => random_password, :port => '-1' }
 }

end

–
1.7.2.5

ohadlevy · July 19, 2012, 6:19am

somehow i missed this patch, blaming traveling can you provide the
context of where you saw an issue with this bug? I thought we already
resolved the issue with memory sizes?

thanks,
Ohad

···

On Mon, Jun 11, 2012 at 12:21 PM, Olivier Favre wrote:

Also divide memory size by 1024 as Libvirt wants KiB, not bytes.

Signed-off-by: Olivier Favre olivier@yakaz.com

…/compute_resources_vms/form/_libvirt.html.erb | 2 ±
lib/foreman/model/libvirt.rb | 11 +++++±----
2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/app/views/compute_resources_vms/form/_libvirt.html.erb
b/app/views/compute_resources_vms/form/_libvirt.html.erb
index 94378f2…3bf833a 100644
— a/app/views/compute_resources_vms/form/_libvirt.html.erb
+++ b/app/views/compute_resources_vms/form/_libvirt.html.erb
@@ -1,7 +1,7 @@
<% new = f.object && f.object.new? -%>
<%= text_f f, :name, :disabled => !new if controller_name != “hosts” %>
<%= selectable_f f, :cpus, 1…compute_resource.max_cpu_count, { }, :class
=> “input-mini”,:disabled => !new %>
-<%= selectable_f f, :memory, memory_options(compute_resource.max_memory),
{ }, :class => “span2”, :disabled => !new %>
+<%= selectable_f f, :memory_size,
memory_options(compute_resource.max_memory), { }, :class => “span2”,
:disabled => !new %>
diff --git a/lib/foreman/model/libvirt.rb b/lib/foreman/model/libvirt.rb index bd21972..79bb8a0 100644 --- a/lib/foreman/model/libvirt.rb +++ b/lib/foreman/model/libvirt.rb @@ -64,6 +64,7 @@ module Foreman::Model end
 def new_vm attr={ }
 attr[:memory_size] = attr[:memory_size].to_i / 1024 if attr.member?
:memory_size
opts = vm_instance_defaults.merge(attr.to_hash).symbolize_keys
   # convert rails nested_attributes into a plain hash
@@ -123,11 +124,11 @@ module Foreman::Model
 def vm_instance_defaults
   {
   :memory     => 768*1024*1024,
   :boot_order => %w[network hd],
   :nics       => [new_nic],
   :volumes    => [new_volume],
   :display    => { :type => 'vnc', :listen => '0', :password =>
random_password, :port => ‘-1’ }
   :memory_size => 768*1024*1024,
   :boot_order  => %w[network hd],
   :nics        => [new_nic],
   :volumes     => [new_volume],
   :display     => { :type => 'vnc', :listen => '0', :password =>
random_password, :port => ‘-1’ }
}
end

–
1.7.2.5

Olivier_Favre · July 19, 2012, 8:55am

No worries.

You already fixed it the day before I sent the patch:
https://github.com/theforeman/foreman/commit/1c81c2b9dd8cda06ef9cf5c2d9a3627a357c2a0d#diff-3
And you fixed it cleanly.

···

2012/7/19 Ohad Levy

On Mon, Jun 11, 2012 at 12:21 PM, Olivier Favre olivier@yakaz.com wrote:
Also divide memory size by 1024 as Libvirt wants KiB, not bytes.

Signed-off-by: Olivier Favre olivier@yakaz.com

…/compute_resources_vms/form/_libvirt.html.erb | 2 ±
lib/foreman/model/libvirt.rb | 11 +++++±----
2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/app/views/compute_resources_vms/form/_libvirt.html.erb
b/app/views/compute_resources_vms/form/_libvirt.html.erb
index 94378f2…3bf833a 100644
— a/app/views/compute_resources_vms/form/_libvirt.html.erb
+++ b/app/views/compute_resources_vms/form/_libvirt.html.erb
@@ -1,7 +1,7 @@
<% new = f.object && f.object.new? -%>
<%= text_f f, :name, :disabled => !new if controller_name != “hosts” %>
<%= selectable_f f, :cpus, 1…compute_resource.max_cpu_count, { },
:class => “input-mini”,:disabled => !new %>
-<%= selectable_f f, :memory,
memory_options(compute_resource.max_memory), { }, :class => “span2”,
:disabled => !new %>
+<%= selectable_f f, :memory_size,
memory_options(compute_resource.max_memory), { }, :class => “span2”,
:disabled => !new %>
diff --git a/lib/foreman/model/libvirt.rb b/lib/foreman/model/libvirt.rb index bd21972..79bb8a0 100644 --- a/lib/foreman/model/libvirt.rb +++ b/lib/foreman/model/libvirt.rb @@ -64,6 +64,7 @@ module Foreman::Model end
 def new_vm attr={ }
 attr[:memory_size] = attr[:memory_size].to_i / 1024 if
attr.member? :memory_size
opts = vm_instance_defaults.merge(attr.to_hash).symbolize_keys
   # convert rails nested_attributes into a plain hash
@@ -123,11 +124,11 @@ module Foreman::Model
 def vm_instance_defaults
   {
   :memory     => 768*1024*1024,
   :boot_order => %w[network hd],
   :nics       => [new_nic],
   :volumes    => [new_volume],
   :display    => { :type => 'vnc', :listen => '0', :password =>
random_password, :port => ‘-1’ }
   :memory_size => 768*1024*1024,
   :boot_order  => %w[network hd],
   :nics        => [new_nic],
   :volumes     => [new_volume],
   :display     => { :type => 'vnc', :listen => '0', :password =>
random_password, :port => ‘-1’ }
}
end

–
1.7.2.5
somehow i missed this patch, blaming traveling can you provide the
context of where you saw an issue with this bug? I thought we already
resolved the issue with memory sizes?

thanks,
Ohad