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 2011/08/02 15:29:21 UTC

[PATCH core 3/5] RHEVM: Added VNC address type for VNC access

From: Michal Fojtik <mf...@redhat.com>


Signed-off-by: Michal fojtik <mf...@redhat.com>
---
 .../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(-)

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
@@ -197,7 +197,7 @@ module RHEVM
   class VM < BaseObject
     attr_reader :description, :status, :memory, :profile, :display, :host, :cluster, :template, :macs
     attr_reader :storage, :cores, :username, :creation_time
-    attr_reader :ip
+    attr_reader :ip, :vnc
 
     def initialize(client, xml)
       super(client, xml[:id], xml[:href], (xml/'name').first.text)
@@ -227,6 +227,10 @@ module RHEVM
       @macs = (xml/'nics/nic/mac').collect { |mac| mac[:address] }
       @creation_time = (xml/'creation_time').text
       @ip = ((xml/'guest_info/ip').first[:address] rescue nil)
+      unless @ip
+        @vnc = ((xml/'display/address').first.text rescue "127.0.0.1")
+        @vnc += ":#{((xml/'display/port').first.text rescue "5890")}"
+      end
     end
 
   end
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
-- 
1.7.4.1


Re: [PATCH core 3/5] RHEVM: Added VNC address type for VNC access

Posted by David Lutterkort <lu...@redhat.com>.
On Tue, 2011-08-02 at 15:29 +0200, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
> 
> 
> Signed-off-by: Michal fojtik <mf...@redhat.com>
> ---
>  .../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