You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by mf...@redhat.com on 2012/02/21 15:21:30 UTC
[PATCH core 1/2] CIMI: MachineConfiguration added the default value for non-static hardware_profiles
From: Michal Fojtik <mf...@redhat.com>
Signed-off-by: Michal fojtik <mf...@redhat.com>
---
server/lib/cimi/model/machine_configuration.rb | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/server/lib/cimi/model/machine_configuration.rb b/server/lib/cimi/model/machine_configuration.rb
index d36ebdf..feab4e8 100644
--- a/server/lib/cimi/model/machine_configuration.rb
+++ b/server/lib/cimi/model/machine_configuration.rb
@@ -57,8 +57,8 @@ class CIMI::Model::MachineConfiguration < CIMI::Model::Base
"of memory and #{profile.cpu.value} CPU",
:cpu => profile.cpu.value,
:created => Time.now.to_s, # FIXME: DC hardware_profile has no mention about created_at
- :memory => { :quantity => profile.memory.value, :units => profile.memory.unit },
- :disks => [ { :capacity => { :quantity => profile.storage.value, :units => profile.storage.unit } } ],
+ :memory => { :quantity => profile.memory.value || profile.memory.default, :units => profile.memory.unit },
+ :disks => [ { :capacity => { :quantity => profile.storage.value || profile.storage.default, :units => profile.storage.unit } } ],
:uri => context.machine_configuration_url(profile.name)
}
self.new(machine_hash)
--
1.7.9.1
Re: [PATCH core 2/2] RHEV-M: Handled exception when backend does not support MachineAdmin. The ignored 'name' parameter for Machine was fixed
Posted by Michal Fojtik <mf...@redhat.com>.
Hi,
Sure, I'll rebase and split this to smaller commits with proper message.
Thanks for pointing this out :-)
-- Michal
Michal Fojtik
http://deltacloud.org
mfojtik@redhat.com
On Feb 22, 2012, at 2:03 AM, David Lutterkort wrote:
> On Tue, 2012-02-21 at 15:21 +0100, mfojtik@redhat.com wrote:
>> From: Michal Fojtik <mf...@redhat.com>
>
> Why does the commit message say 'RHEV-M' ? The patch doesn't touch the
> RHEV-M driver. 'CIMI' seems more appropriate.
>
> Couple more comments:
>
>> diff --git a/clients/cimi/lib/entities/machine.rb b/clients/cimi/lib/entities/machine.rb
>> index d70d0e9..c17925b 100644
>> --- a/clients/cimi/lib/entities/machine.rb
>> +++ b/clients/cimi/lib/entities/machine.rb
>> @@ -86,10 +91,10 @@ class CIMI::Frontend::Machine < CIMI::Frontend::Entity
>> xml.Machine(:xmlns => CIMI::Frontend::CMWG_NAMESPACE) {
>> xml.name params[:machine][:name]
>> xml.description params[:machine][:description]
>> - xml.MachineTemplate {
>> - xml.MachineConfig( :href => params[:machine][:machine_configuration] )
>> - xml.MachineImage( :href => params[:machine][:machine_image] )
>> - xml.MachineAdmin( :href => params[:machine][:machine_admin] ) unless params[:machine][:machine_admin].empty?
>> + xml.machineTemplate {
>> + xml.machineConfig( :href => params[:machine][:machine_configuration] )
>> + xml.machineImage( :href => params[:machine][:machine_image] )
>> + xml.machineAdmin( :href => params[:machine][:machine_admin] ) unless params[:machine][:machine_admin].nil?
>
> This hunk doesn't seem to have anything to do with exception handling;
> should really go into its own patch. (And then I wouldn't have to stare
> at it to figure out it's about changing the capitalization of tags;
> still don't know why the change from empty? to nil?)
>
>> diff --git a/server/lib/cimi/model/machine.rb b/server/lib/cimi/model/machine.rb
>> index 949a072..88c9f73 100644
>> --- a/server/lib/cimi/model/machine.rb
>> +++ b/server/lib/cimi/model/machine.rb
>> @@ -82,6 +82,7 @@ class CIMI::Model::Machine < CIMI::Model::Base
>> hardware_profile_id = machine_template['machineConfig'][0]["href"].split('/').last
>> image_id = machine_template['machineImage'][0]["href"].split('/').last
>> additional_params = {}
>> + additional_params[:name] =xml['name'][0] if xml['name']
>
> This also doesn't seem to be related to MachineAdmin (the reason I harp
> on this is that when I write the NEWS for the next release, changes like
> this just disappear)
>
>> diff --git a/server/lib/deltacloud/backend_capability.rb b/server/lib/deltacloud/backend_capability.rb
>> index 57e8cd3..05e193c 100644
>> --- a/server/lib/deltacloud/backend_capability.rb
>> +++ b/server/lib/deltacloud/backend_capability.rb
>> @@ -37,7 +37,8 @@ module Deltacloud::BackendCapability
>>
>> def check_capability(backend)
>> if !has_capability?(backend)
>> - raise Failure.new(capability, "#{capability} capability not supported by backend #{backend.class.name}")
>> + e = Failure.new(capability, "#{capability} capability not supported by backend #{backend.class.name}")
>> + raise Deltacloud::ExceptionHandler::NotSupported.new(e, nil)
>> end
>> end
>>
>> diff --git a/server/lib/deltacloud/base_driver/exceptions.rb b/server/lib/deltacloud/base_driver/exceptions.rb
>> index e44c89b..3969e7e 100644
>> --- a/server/lib/deltacloud/base_driver/exceptions.rb
>> +++ b/server/lib/deltacloud/base_driver/exceptions.rb
>> @@ -79,6 +79,13 @@ module Deltacloud
>> end
>> end
>>
>> + class NotSupported < DeltacloudException
>> + def initialize(e, message)
>> + message ||= e.message
>> + super(501, e.class.name, message, e.backtrace)
>> + end
>> + end
>
> Why create the Failure exception, just to take it apart and not use it ?
>
> David
>
>
Re: [PATCH core 2/2] RHEV-M: Handled exception when backend does
not support MachineAdmin. The ignored 'name' parameter for Machine was fixed
Posted by David Lutterkort <lu...@redhat.com>.
On Tue, 2012-02-21 at 15:21 +0100, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
Why does the commit message say 'RHEV-M' ? The patch doesn't touch the
RHEV-M driver. 'CIMI' seems more appropriate.
Couple more comments:
> diff --git a/clients/cimi/lib/entities/machine.rb b/clients/cimi/lib/entities/machine.rb
> index d70d0e9..c17925b 100644
> --- a/clients/cimi/lib/entities/machine.rb
> +++ b/clients/cimi/lib/entities/machine.rb
> @@ -86,10 +91,10 @@ class CIMI::Frontend::Machine < CIMI::Frontend::Entity
> xml.Machine(:xmlns => CIMI::Frontend::CMWG_NAMESPACE) {
> xml.name params[:machine][:name]
> xml.description params[:machine][:description]
> - xml.MachineTemplate {
> - xml.MachineConfig( :href => params[:machine][:machine_configuration] )
> - xml.MachineImage( :href => params[:machine][:machine_image] )
> - xml.MachineAdmin( :href => params[:machine][:machine_admin] ) unless params[:machine][:machine_admin].empty?
> + xml.machineTemplate {
> + xml.machineConfig( :href => params[:machine][:machine_configuration] )
> + xml.machineImage( :href => params[:machine][:machine_image] )
> + xml.machineAdmin( :href => params[:machine][:machine_admin] ) unless params[:machine][:machine_admin].nil?
This hunk doesn't seem to have anything to do with exception handling;
should really go into its own patch. (And then I wouldn't have to stare
at it to figure out it's about changing the capitalization of tags;
still don't know why the change from empty? to nil?)
> diff --git a/server/lib/cimi/model/machine.rb b/server/lib/cimi/model/machine.rb
> index 949a072..88c9f73 100644
> --- a/server/lib/cimi/model/machine.rb
> +++ b/server/lib/cimi/model/machine.rb
> @@ -82,6 +82,7 @@ class CIMI::Model::Machine < CIMI::Model::Base
> hardware_profile_id = machine_template['machineConfig'][0]["href"].split('/').last
> image_id = machine_template['machineImage'][0]["href"].split('/').last
> additional_params = {}
> + additional_params[:name] =xml['name'][0] if xml['name']
This also doesn't seem to be related to MachineAdmin (the reason I harp
on this is that when I write the NEWS for the next release, changes like
this just disappear)
> diff --git a/server/lib/deltacloud/backend_capability.rb b/server/lib/deltacloud/backend_capability.rb
> index 57e8cd3..05e193c 100644
> --- a/server/lib/deltacloud/backend_capability.rb
> +++ b/server/lib/deltacloud/backend_capability.rb
> @@ -37,7 +37,8 @@ module Deltacloud::BackendCapability
>
> def check_capability(backend)
> if !has_capability?(backend)
> - raise Failure.new(capability, "#{capability} capability not supported by backend #{backend.class.name}")
> + e = Failure.new(capability, "#{capability} capability not supported by backend #{backend.class.name}")
> + raise Deltacloud::ExceptionHandler::NotSupported.new(e, nil)
> end
> end
>
> diff --git a/server/lib/deltacloud/base_driver/exceptions.rb b/server/lib/deltacloud/base_driver/exceptions.rb
> index e44c89b..3969e7e 100644
> --- a/server/lib/deltacloud/base_driver/exceptions.rb
> +++ b/server/lib/deltacloud/base_driver/exceptions.rb
> @@ -79,6 +79,13 @@ module Deltacloud
> end
> end
>
> + class NotSupported < DeltacloudException
> + def initialize(e, message)
> + message ||= e.message
> + super(501, e.class.name, message, e.backtrace)
> + end
> + end
Why create the Failure exception, just to take it apart and not use it ?
David
[PATCH core 2/2] RHEV-M: Handled exception when backend does not support MachineAdmin. The ignored 'name' parameter for Machine was fixed
Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>
Signed-off-by: Michal fojtik <mf...@redhat.com>
---
clients/cimi/lib/entities/machine.rb | 17 +++++++++++------
clients/cimi/views/machines/new.haml | 9 ++++++---
server/lib/cimi/model/machine.rb | 1 +
server/lib/deltacloud/backend_capability.rb | 3 ++-
server/lib/deltacloud/base_driver/exceptions.rb | 7 +++++++
5 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/clients/cimi/lib/entities/machine.rb b/clients/cimi/lib/entities/machine.rb
index d70d0e9..c17925b 100644
--- a/clients/cimi/lib/entities/machine.rb
+++ b/clients/cimi/lib/entities/machine.rb
@@ -20,8 +20,13 @@ class CIMI::Frontend::Machine < CIMI::Frontend::Entity
@machine_images = CIMI::Model::MachineImageCollection.from_xml(machine_image_xml)
machine_conf_xml = get_entity_collection('machine_configurations', credentials)
@machine_configurations = CIMI::Model::MachineConfigurationCollection.from_xml(machine_conf_xml)
- machine_admins_xml = get_entity_collection('machine_admins', credentials)
- @machine_admins = CIMI::Model::MachineAdminCollection.from_xml(machine_admins_xml)
+ begin
+ machine_admins_xml = get_entity_collection('machine_admins', credentials)
+ @machine_admins = CIMI::Model::MachineAdminCollection.from_xml(machine_admins_xml)
+ # In case backend does not support MachineAdmin collection
+ rescue RestClient::InternalServerError
+ @machine_admins = []
+ end
haml :'machines/new'
end
@@ -86,10 +91,10 @@ class CIMI::Frontend::Machine < CIMI::Frontend::Entity
xml.Machine(:xmlns => CIMI::Frontend::CMWG_NAMESPACE) {
xml.name params[:machine][:name]
xml.description params[:machine][:description]
- xml.MachineTemplate {
- xml.MachineConfig( :href => params[:machine][:machine_configuration] )
- xml.MachineImage( :href => params[:machine][:machine_image] )
- xml.MachineAdmin( :href => params[:machine][:machine_admin] ) unless params[:machine][:machine_admin].empty?
+ xml.machineTemplate {
+ xml.machineConfig( :href => params[:machine][:machine_configuration] )
+ xml.machineImage( :href => params[:machine][:machine_image] )
+ xml.machineAdmin( :href => params[:machine][:machine_admin] ) unless params[:machine][:machine_admin].nil?
}
}
end.to_xml
diff --git a/clients/cimi/views/machines/new.haml b/clients/cimi/views/machines/new.haml
index 984ef9b..8de1fe1 100644
--- a/clients/cimi/views/machines/new.haml
+++ b/clients/cimi/views/machines/new.haml
@@ -43,9 +43,12 @@
Machine Admin
%div.input
%select{ :name => 'machine[machine_admin]' }
- %option{ :value => '', :selected => :selected }
- - @machine_admins.machine_admins.each do |admin|
- %option{ :value => admin.href }=href_to_id(admin.href)
+ - unless @machine_admins.empty?
+ %option{ :value => '', :selected => :selected }
+ - @machine_admins.machine_admins.each do |admin|
+ %option{ :value => admin.href }=href_to_id(admin.href)
+ - else
+ %option{ :disabled => 'disabled'}="Not supported"
%div.actions
%input{ :type => :submit, :class => 'btn primary', :value => "Create machine" }
%button{ :type => :reset, :class => 'btn' } Reset
diff --git a/server/lib/cimi/model/machine.rb b/server/lib/cimi/model/machine.rb
index 949a072..88c9f73 100644
--- a/server/lib/cimi/model/machine.rb
+++ b/server/lib/cimi/model/machine.rb
@@ -82,6 +82,7 @@ class CIMI::Model::Machine < CIMI::Model::Base
hardware_profile_id = machine_template['machineConfig'][0]["href"].split('/').last
image_id = machine_template['machineImage'][0]["href"].split('/').last
additional_params = {}
+ additional_params[:name] =xml['name'][0] if xml['name']
if machine_template.has_key? 'machineAdmin'
additional_params[:keyname] = machine_template['machineAdmin'][0]["href"].split('/').last
end
diff --git a/server/lib/deltacloud/backend_capability.rb b/server/lib/deltacloud/backend_capability.rb
index 57e8cd3..05e193c 100644
--- a/server/lib/deltacloud/backend_capability.rb
+++ b/server/lib/deltacloud/backend_capability.rb
@@ -37,7 +37,8 @@ module Deltacloud::BackendCapability
def check_capability(backend)
if !has_capability?(backend)
- raise Failure.new(capability, "#{capability} capability not supported by backend #{backend.class.name}")
+ e = Failure.new(capability, "#{capability} capability not supported by backend #{backend.class.name}")
+ raise Deltacloud::ExceptionHandler::NotSupported.new(e, nil)
end
end
diff --git a/server/lib/deltacloud/base_driver/exceptions.rb b/server/lib/deltacloud/base_driver/exceptions.rb
index e44c89b..3969e7e 100644
--- a/server/lib/deltacloud/base_driver/exceptions.rb
+++ b/server/lib/deltacloud/base_driver/exceptions.rb
@@ -79,6 +79,13 @@ module Deltacloud
end
end
+ class NotSupported < DeltacloudException
+ def initialize(e, message)
+ message ||= e.message
+ super(501, e.class.name, message, e.backtrace)
+ end
+ end
+
class ExceptionDef
attr_accessor :status
attr_accessor :message
--
1.7.9.1
Re: [PATCH core 1/2] CIMI: MachineConfiguration added the default
value for non-static hardware_profiles
Posted by David Lutterkort <lu...@redhat.com>.
On Tue, 2012-02-21 at 15:21 +0100, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
ACK; minor nit: please try to keep the first line of a commit message to
less than 80 chars. Additional info should go into the body of the
commit message.
David