providers/virtualbox: tidying up the linked clone feature

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

plugins/providers/virtualbox/action.rb

@@ -328,12 +328,13 @@ module VagrantPlugins
               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
+
               b2.use MatchMACAddress
             end
           end

plugins/providers/virtualbox/action/create_clone.rb

@@ -1,5 +1,4 @@
 require "log4r"
-#require "lockfile"
 
 module VagrantPlugins
   module ProviderVirtualBox
@@ -14,12 +13,14 @@ module VagrantPlugins
           @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

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

plugins/providers/virtualbox/action/import_master.rb

@@ -1,5 +1,6 @@
 require "log4r"
-#require "lockfile"
+
+require "digest/md5"
 
 module VagrantPlugins
   module ProviderVirtualBox
@@ -11,43 +12,12 @@ module VagrantPlugins
         end
 
         def call(env)
-          master_id_file = env[:machine].box.directory.join("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
-          env[:machine].env.lock(Digest::MD5.hexdigest(env[:machine].box.name), retry: true) do
+          # have a master that exists.
-            env[:master_id] = master_id_file.read.chomp if master_id_file.file?          
+          lock_key = Digest::MD5.hexdigest(env[:machine].box.name)
-            if env[:master_id] && env[:machine].provider.driver.vm_exists?(env[:master_id])
+          env[:machine].env.lock(lock_key, retry: true) do
-              # Master VM already exists -> nothing to do - continue.
+            import_master(env)
-              @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]
-
-            @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
@@ -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

plugins/providers/virtualbox/config.rb

@@ -36,7 +36,7 @@ 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

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...
+          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}.