From 2c936b2e3722a84ca703e335e8cb2f19c09d670f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 6 Oct 2015 14:11:41 -0400 Subject: [PATCH] providers/virtualbox: tidying up the linked clone feature --- lib/vagrant/environment.rb | 4 +- plugins/providers/virtualbox/action.rb | 7 +- .../virtualbox/action/create_clone.rb | 9 +- plugins/providers/virtualbox/action/import.rb | 5 +- .../virtualbox/action/import_master.rb | 88 +++++++++++-------- plugins/providers/virtualbox/config.rb | 8 +- templates/locales/en.yml | 9 +- 7 files changed, 75 insertions(+), 55 deletions(-) diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index d4b8db1fd..c3dabd676 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -529,7 +529,9 @@ module Vagrant begin File.delete(lock_path) rescue - @logger.debug("Failed to delete lock file #{lock_path} - some other thread might be trying to acquire it -> ignoring this error") + @logger.error( + "Failed to delete lock file #{lock_path} - some other thread " + + "might be trying to acquire it. ignoring this error") end end end diff --git a/plugins/providers/virtualbox/action.rb b/plugins/providers/virtualbox/action.rb index 635907827..088221ed1 100644 --- a/plugins/providers/virtualbox/action.rb +++ b/plugins/providers/virtualbox/action.rb @@ -327,13 +327,14 @@ module VagrantPlugins if !env[:result] b2.use CheckAccessible b2.use Customize, "pre-import" - - if env[:machine].provider_config.use_linked_clone + + if env[:machine].provider_config.linked_clone b2.use ImportMaster b2.use CreateClone else b2.use Import - end + end + b2.use MatchMACAddress end end diff --git a/plugins/providers/virtualbox/action/create_clone.rb b/plugins/providers/virtualbox/action/create_clone.rb index e26b5721f..9cc8f7ed5 100644 --- a/plugins/providers/virtualbox/action/create_clone.rb +++ b/plugins/providers/virtualbox/action/create_clone.rb @@ -1,5 +1,4 @@ require "log4r" -#require "lockfile" module VagrantPlugins module ProviderVirtualBox @@ -12,14 +11,16 @@ module VagrantPlugins def call(env) @logger.info("Creating linked clone from master '#{env[:master_id]}'") - + env[:ui].info I18n.t("vagrant.actions.vm.clone.creating", name: env[:machine].box.name) - env[:machine].id = env[:machine].provider.driver.clonevm(env[:master_id], env[:machine].box.name, "base") do |progress| + env[:machine].id = env[:machine].provider.driver.clonevm( + env[:master_id], env[:machine].box.name, "base") do |progress| env[:ui].clear_line env[:ui].report_progress(progress, 100, false) end - # Clear the line one last time since the progress meter doesn't disappear immediately. + # Clear the line one last time since the progress meter doesn't + # disappear immediately. env[:ui].clear_line # Flag as erroneous and return if clone failed diff --git a/plugins/providers/virtualbox/action/import.rb b/plugins/providers/virtualbox/action/import.rb index 6648faefc..238f88e00 100644 --- a/plugins/providers/virtualbox/action/import.rb +++ b/plugins/providers/virtualbox/action/import.rb @@ -12,11 +12,14 @@ module VagrantPlugins # Import the virtual machine ovf_file = env[:machine].box.directory.join("box.ovf").to_s - env[:machine].id = env[:machine].provider.driver.import(ovf_file) do |progress| + id = env[:machine].provider.driver.import(ovf_file) do |progress| env[:ui].clear_line env[:ui].report_progress(progress, 100, false) end + # Set the machine ID + env[:machine].id = id if !env[:skip_machine] + # Clear the line one last time since the progress meter doesn't disappear # immediately. env[:ui].clear_line diff --git a/plugins/providers/virtualbox/action/import_master.rb b/plugins/providers/virtualbox/action/import_master.rb index 61b0064eb..9b088b2a1 100644 --- a/plugins/providers/virtualbox/action/import_master.rb +++ b/plugins/providers/virtualbox/action/import_master.rb @@ -1,5 +1,6 @@ require "log4r" -#require "lockfile" + +require "digest/md5" module VagrantPlugins module ProviderVirtualBox @@ -11,45 +12,14 @@ module VagrantPlugins end def call(env) - master_id_file = env[:machine].box.directory.join("master_id") - - env[:machine].env.lock(Digest::MD5.hexdigest(env[:machine].box.name), retry: true) do - env[:master_id] = master_id_file.read.chomp if master_id_file.file? - if env[:master_id] && env[:machine].provider.driver.vm_exists?(env[:master_id]) - # Master VM already exists -> nothing to do - continue. - @logger.info("Master VM for '#{env[:machine].box.name}' already exists (id=#{env[:master_id]}) - skipping import step.") - return @app.call(env) - end - - env[:ui].info I18n.t("vagrant.actions.vm.clone.importing", name: env[:machine].box.name) - - # Import the virtual machine - ovf_file = env[:machine].box.directory.join("box.ovf").to_s - env[:master_id] = env[:machine].provider.driver.import(ovf_file) do |progress| - env[:ui].clear_line - env[:ui].report_progress(progress, 100, false) - end - - # Clear the line one last time since the progress meter doesn't disappear immediately. - env[:ui].clear_line - - # Flag as erroneous and return if import failed - raise Vagrant::Errors::VMImportFailure if !env[:master_id] + # Do the import while locked so that nobody else imports + # a master at the same time. This is a no-op if we already + # have a master that exists. + lock_key = Digest::MD5.hexdigest(env[:machine].box.name) + env[:machine].env.lock(lock_key, retry: true) do + import_master(env) + end - @logger.info("Imported box #{env[:machine].box.name} as master vm with id #{env[:master_id]}") - - @logger.info("Creating base snapshot for master VM.") - env[:machine].provider.driver.create_snapshot(env[:master_id], "base") do |progress| - env[:ui].clear_line - env[:ui].report_progress(progress, 100, false) - end - - @logger.debug("Writing id of master VM '#{env[:master_id]}' to #{master_id_file}") - master_id_file.open("w+") do |f| - f.write(env[:master_id]) - end - end - # If we got interrupted, then the import could have been # interrupted and its not a big deal. Just return out. if env[:interrupted] @@ -60,6 +30,46 @@ module VagrantPlugins # Import completed successfully. Continue the chain @app.call(env) end + + protected + + def import_master(env) + master_id_file = env[:machine].box.directory.join("master_id") + + # Read the master ID if we have it in the file. + env[:master_id] = master_id_file.read.chomp if master_id_file.file? + + # If we have the ID and the VM exists already, then we + # have nothing to do. Success! + if env[:master_id] && env[:machine].provider.driver.vm_exists?(env[:master_id]) + @logger.info( + "Master VM for '#{env[:machine].box.name}' already exists " + + " (id=#{env[:master_id]}) - skipping import step.") + return + end + + env[:ui].info(I18n.t("vagrant.actions.vm.clone.setup_master")) + env[:ui].detail("\n"+I18n.t("vagrant.actions.vm.clone.setup_master_detail")) + + # Import the virtual machine + import_env = env[:action_runner].run(Import, skip_machine: true) + env[:master_id] = import_env[:machine_id] + + @logger.info( + "Imported box #{env[:machine].box.name} as master vm " + + "with id #{env[:master_id]}") + + @logger.info("Creating base snapshot for master VM.") + env[:machine].provider.driver.create_snapshot(env[:master_id], "base") do |progress| + env[:ui].clear_line + env[:ui].report_progress(progress, 100, false) + end + + @logger.debug("Writing id of master VM '#{env[:master_id]}' to #{master_id_file}") + master_id_file.open("w+") do |f| + f.write(env[:master_id]) + end + end end end end diff --git a/plugins/providers/virtualbox/config.rb b/plugins/providers/virtualbox/config.rb index 55b8356aa..12b882b01 100644 --- a/plugins/providers/virtualbox/config.rb +++ b/plugins/providers/virtualbox/config.rb @@ -36,8 +36,8 @@ module VagrantPlugins # VM generated from the specified box. # # @return [Boolean] - attr_accessor :use_linked_clone - + attr_accessor :linked_clone + # This should be set to the name of the machine in the VirtualBox # GUI. # @@ -65,7 +65,7 @@ module VagrantPlugins @name = UNSET_VALUE @network_adapters = {} @gui = UNSET_VALUE - @use_linked_clone = UNSET_VALUE + @linked_clone = UNSET_VALUE # We require that network adapter 1 is a NAT device. network_adapter(1, :nat) @@ -144,7 +144,7 @@ module VagrantPlugins @gui = false if @gui == UNSET_VALUE # Do not create linked clone by default - @use_linked_clone = false if @use_linked_clone == UNSET_VALUE + @linked_clone = false if @linked_clone == UNSET_VALUE # The default name is just nothing, and we default it @name = nil if @name == UNSET_VALUE diff --git a/templates/locales/en.yml b/templates/locales/en.yml index d0f311fd0..171cd60ba 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1619,10 +1619,13 @@ en: clear_shared_folders: deleting: Cleaning previously set shared folders... clone: - importing: Importing box '%{name}' as master vm... - creating: Creating linked clone... + setup_master: Preparing master VM for linked clones... + setup_master_detail: |- + This is a one time operation. Once the master VM is prepared, + it will be used as a base for linked clones, making the creation + of new VMs take milliseconds on a modern system. + creating: Creating linked clone... failure: Creation of the linked clone failed. - create_master: failure: |- Failed to create lock-file for master VM creation for box %{box}.