From a1b66f82aa3cde4da2f39021d1ba2a9e897fecc5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 3 Jan 2012 10:34:35 -0800 Subject: [PATCH] Consistently generate names for forwarded ports. To do this, I convert the ports to base 32 strings in the format of "guestport-hostport." This makes a consistent mapping we can use to look up if the forwarded port is set. --- .../action/vm/check_port_collisions.rb | 20 ++++++++++--------- lib/vagrant/action/vm/forward_ports.rb | 16 +++------------ lib/vagrant/config/vm.rb | 8 +++----- templates/locales/en.yml | 9 +++++---- 4 files changed, 22 insertions(+), 31 deletions(-) diff --git a/lib/vagrant/action/vm/check_port_collisions.rb b/lib/vagrant/action/vm/check_port_collisions.rb index 22c2e7eec..ab5a92266 100644 --- a/lib/vagrant/action/vm/check_port_collisions.rb +++ b/lib/vagrant/action/vm/check_port_collisions.rb @@ -23,15 +23,15 @@ module Vagrant end existing = env[:vm].driver.read_used_ports - env[:vm].config.vm.forwarded_ports.each do |name, options| + env[:vm].config.vm.forwarded_ports.each do |options| # Use the proper port, whether that be the configured port or the # port that is currently on use of the VM. hostport = options[:hostport].to_i - hostport = current[name] if current.has_key?(name) + hostport = current[options[:name]] if current.has_key?(options[:name]) if existing.include?(hostport) # We have a collision! Handle it - send("handle_#{handler}".to_sym, name, options, existing) + send("handle_#{handler}".to_sym, options, existing) end end @@ -39,17 +39,19 @@ module Vagrant end # Handles a port collision by raising an exception. - def handle_error(name, options, existing_ports) + def handle_error(options, existing_ports) raise Errors::ForwardPortCollisionResume end # Handles a port collision by attempting to fix it. - def handle_correct(name, options, existing_ports) + def handle_correct(options, existing_ports) + # We need to keep this for messaging purposes + original_hostport = options[:hostport] + if !options[:auto] # Auto fixing is disabled for this port forward, so we # must throw an error so the user can fix it. - raise Errors::ForwardPortCollision, :name => name, - :host_port => options[:hostport].to_s, + raise Errors::ForwardPortCollision, :host_port => options[:hostport].to_s, :guest_port => options[:guestport].to_s end @@ -62,7 +64,6 @@ module Vagrant if range.empty? raise Errors::ForwardPortAutolistEmpty, :vm_name => @env[:vm].name, - :name => name, :host_port => options[:hostport].to_s, :guest_port => options[:guestport].to_s end @@ -74,7 +75,8 @@ module Vagrant # Notify the user @env[:ui].info(I18n.t("vagrant.actions.vm.forward_ports.fixed_collision", - :name => name, + :host_port => original_hostport.to_s, + :guest_port => options[:guestport].to_s, :new_port => options[:hostport])) end end diff --git a/lib/vagrant/action/vm/forward_ports.rb b/lib/vagrant/action/vm/forward_ports.rb index db6dd2ef2..fe55b2514 100644 --- a/lib/vagrant/action/vm/forward_ports.rb +++ b/lib/vagrant/action/vm/forward_ports.rb @@ -24,7 +24,7 @@ module Vagrant # This method checks for any forwarded ports on the host below # 1024, which causes the forwarded ports to fail. def threshold_check - @env[:vm].config.vm.forwarded_ports.each do |name, options| + @env[:vm].config.vm.forwarded_ports.each do |options| if options[:hostport] <= 1024 @env[:ui].warn I18n.t("vagrant.actions.vm.forward_ports.privileged_ports") return @@ -33,12 +33,11 @@ module Vagrant end def forward_ports(vm) - counter = 0 ports = [] interfaces = @env[:vm].driver.read_network_interfaces - @env[:vm].config.vm.forwarded_ports.each do |name, options| + @env[:vm].config.vm.forwarded_ports.each do |options| message_attributes = { :guest_port => options[:guestport], :host_port => options[:hostport], @@ -60,17 +59,8 @@ module Vagrant next end - # VirtualBox requires a unique name for the forwarded port, so we - # will give it one. - name = options[:name] - if !name - # Auto-generate the name based on a counter of every forwarded port - name = "fp#{counter}" - counter += 1 - end - # Add the options to the ports array to send to the driver later - ports << options.merge(:name => name, :adapter => options[:adapter]) + ports << options.merge(:name => options[:name], :adapter => options[:adapter]) end @env[:vm].driver.forward_ports(ports) diff --git a/lib/vagrant/config/vm.rb b/lib/vagrant/config/vm.rb index d9aacd3ae..047dc4171 100644 --- a/lib/vagrant/config/vm.rb +++ b/lib/vagrant/config/vm.rb @@ -19,7 +19,7 @@ module Vagrant attr_accessor :guest def initialize - @forwarded_ports = {} + @forwarded_ports = [] @shared_folders = {} @networks = [] @provisioners = [] @@ -40,16 +40,14 @@ Please change your configurations to match this new syntax. MESSAGE end - options = { - :name => nil, + @forwarded_ports << { + :name => "#{guestport.to_s(32)}-#{hostport.to_s(32)}", :guestport => guestport, :hostport => hostport, :protocol => :tcp, :adapter => 1, :auto => false }.merge(options || {}) - - forwarded_ports[name.to_s] = options end def share_folder(name, guestpath, hostpath, opts=nil) diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 6e87ff269..5d8f19241 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -355,19 +355,20 @@ en: auto-correction range are all also used. VM: %{vm_name} - Forwarded port: %{name} (%{guest_port} => %{host_port}) + Forwarded port: %{guest_port} => %{host_port} collision_error: |- Vagrant cannot forward the specified ports on this VM, since they would collide with another VirtualBox virtual machine's forwarded - ports! The '%{name}' forwarded port (%{host_port}) is already in use on the host + ports! The forwarded port to %{host_port} is already in use on the host machine. To fix this, modify your current projects Vagrantfile to use another port. Example, where '1234' would be replaced by a unique host port: - config.vm.forward_port("%{name}", %{guest_port}, 1234) + config.vm.forward_port %{guest_port}, 1234 - fixed_collision: Fixed port collision '%{name}'. Now on port %{new_port}. + fixed_collision: |- + Fixed port collision for %{guest_port} => %{host_port}. Now on port %{new_port}. forwarding: Forwarding ports... forwarding_entry: |- -- %{guest_port} => %{host_port} (adapter %{adapter})