On Tue, 2011-08-02 at 15:29 +0200, [email protected] wrote:
> From: Michal Fojtik <[email protected]>
>
>
> Signed-off-by: Michal fojtik <[email protected]>
> ---
> .../lib/deltacloud/drivers/rhevm/rhevm_client.rb | 6 +++++-
> .../lib/deltacloud/drivers/rhevm/rhevm_driver.rb | 1 +
> .../lib/deltacloud/helpers/application_helper.rb | 1 +
> 3 files changed, 7 insertions(+), 1 deletions(-)
ACK. One comment:
> diff --git a/server/lib/deltacloud/drivers/rhevm/rhevm_client.rb
> b/server/lib/deltacloud/drivers/rhevm/rhevm_client.rb
> index cadaaff..c5ffebe 100644
> --- a/server/lib/deltacloud/drivers/rhevm/rhevm_client.rb
> +++ b/server/lib/deltacloud/drivers/rhevm/rhevm_client.rb
>
> diff --git a/server/lib/deltacloud/drivers/rhevm/rhevm_driver.rb
> b/server/lib/deltacloud/drivers/rhevm/rhevm_driver.rb
> index c0087e8..832620a 100644
> --- a/server/lib/deltacloud/drivers/rhevm/rhevm_driver.rb
> +++ b/server/lib/deltacloud/drivers/rhevm/rhevm_driver.rb
> @@ -213,6 +213,7 @@ class RHEVMDriver < Deltacloud::BaseDriver
> # If everything fails fallback to report MAC address
> public_addresses = inst.macs if public_addresses.empty?
> public_addresses.flatten!
> + public_addresses << inst.vnc if inst.vnc
> Instance.new(
> :id => inst.id,
> :name => inst.name,
> diff --git a/server/lib/deltacloud/helpers/application_helper.rb
> b/server/lib/deltacloud/helpers/application_helper.rb
> index 37bfec4..4b13d48 100644
> --- a/server/lib/deltacloud/helpers/application_helper.rb
> +++ b/server/lib/deltacloud/helpers/application_helper.rb
> @@ -250,6 +250,7 @@ module ApplicationHelper
> def address_type(address)
> case address
> when /^(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})?$/; :ipv4
> + when /^(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})?:([\-\d]+)$/; :vnc
> when /^(\S{1,2}:\S{1,2}:\S{1,2}:\S{1,2}:\S{1,2}:\S{1,2})?$/; :mac
> else :hostname
> end
It would be cleaner if we stored the type of address explicitly in
public_addresses instead of guessing it; especially since now a RHEV-M
vnc address in theory could look like type ipv4.
Instead of pushing just the address onto public_addresses, we should
just push a pair [type, address], i.e. [:vnc, "172.16.0.1"] or a hash
{ :vnc => "172.16.0.1" }
Since this will be fairly invasive across all drivers, this should wait
until after the next release. When you commit the patch, just put a
FIXME comment into address_type
David
