From 2e64aec89840b62118c0349a7683fb9382eddbad Mon Sep 17 00:00:00 2001 From: Mikhail Zholobov Date: Tue, 12 Jan 2016 15:59:28 +0200 Subject: [PATCH 1/4] action/box_register: Remove curly brackets from the box UUID Following the RFC 4122, there should not be any brackets in UUID string --- lib/vagrant-parallels/action/box_register.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vagrant-parallels/action/box_register.rb b/lib/vagrant-parallels/action/box_register.rb index 8ad4530a..74e59288 100644 --- a/lib/vagrant-parallels/action/box_register.rb +++ b/lib/vagrant-parallels/action/box_register.rb @@ -58,7 +58,7 @@ def box_id(env, box_path) config: tpl_config end - id + id.delete('{}') end def register_box(env) From 917b2f387c62afb526fa6ea1de910e4cd5c239de Mon Sep 17 00:00:00 2001 From: Mikhail Zholobov Date: Tue, 12 Jan 2016 21:47:24 +0200 Subject: [PATCH 2/4] Use temporary lock file for boxes to prevent early unregister in the race In parallel run of multi-machine environment the same box image could be cloning simultaneously by more then one envs. So, the box should be unregister by the latest env. To prevent a race condition here, we add a temporary lease file with a counter. It is incremented for incoming envs (new clones) and decremented after the cloning. The last env removes the lock file and unregisters the box. Fixes GH-243 --- lib/vagrant-parallels/action/box_register.rb | 25 +++++++++++- .../action/box_unregister.rb | 23 +++++++++++ lib/vagrant-parallels/driver/base.rb | 40 +------------------ 3 files changed, 48 insertions(+), 40 deletions(-) diff --git a/lib/vagrant-parallels/action/box_register.rb b/lib/vagrant-parallels/action/box_register.rb index 74e59288..c8a57d19 100644 --- a/lib/vagrant-parallels/action/box_register.rb +++ b/lib/vagrant-parallels/action/box_register.rb @@ -1,3 +1,4 @@ +require 'fileutils' require 'log4r' module VagrantPlugins @@ -61,10 +62,30 @@ def box_id(env, box_path) id.delete('{}') end + def lease_box_lock(env) + lease_file = env[:machine].box.directory.join('box_lease_count') + + # If the temporary file, verify it is not too old. If its older than + # 1 hour, delete it first because previous run may be failed. + if lease_file.file? && lease_file.mtime.to_i < Time.now.to_i - 60 * 60 + lease_file.delete + end + + # Increment a counter in the file. Create the file if it doesn't exist + FileUtils.touch(lease_file) + File.open(lease_file ,'r+') do |file| + num = file.gets.to_i + file.rewind + file.puts num.next + end + end + def register_box(env) - box_id_file = env[:machine].box.directory.join('box_id') + # Increment the lock counter in the temporary lease file + lease_box_lock(env) - # Read the master ID if we have it in the file. + # Read the box ID if we have it in the file. + box_id_file = env[:machine].box.directory.join('box_id') env[:clone_id] = box_id_file.read.chomp if box_id_file.file? # If we have the ID and the VM exists already, then we diff --git a/lib/vagrant-parallels/action/box_unregister.rb b/lib/vagrant-parallels/action/box_unregister.rb index 448a9cf0..17dc0854 100644 --- a/lib/vagrant-parallels/action/box_unregister.rb +++ b/lib/vagrant-parallels/action/box_unregister.rb @@ -31,7 +31,30 @@ def recover(env) private + def release_box_lock(lease_file) + return if !lease_file.file? + + # Decrement the counter in the lease file + File.open(lease_file,'r+') do |file| + num = file.gets.to_i + file.rewind + file.puts(num - 1) + end + + # Delete the lease file if we are the last who need this box. + # Then the box image will be unregistered. + lease_file.delete if lease_file.read.chomp.to_i <= 1 + end + def unregister_box(env) + # Release the box lock + lease_file = env[:machine].box.directory.join('box_lease_count') + release_box_lock(lease_file) + + # Do not unregister the box image if the temporary lease file exists + # Most likely it is cloning to another Vagrant env (in parallel run) + return if lease_file.file? + if env[:clone_id] && env[:machine].provider.driver.vm_exists?(env[:clone_id]) env[:ui].info I18n.t('vagrant_parallels.actions.vm.box.unregister') env[:machine].provider.driver.unregister(env[:clone_id]) diff --git a/lib/vagrant-parallels/driver/base.rb b/lib/vagrant-parallels/driver/base.rb index ae3b9db2..2437a869 100644 --- a/lib/vagrant-parallels/driver/base.rb +++ b/lib/vagrant-parallels/driver/base.rb @@ -529,25 +529,7 @@ def regenerate_src_uuid # @param [String] pvm_file Path to the machine image (*.pvm) # @param [Array] opts List of options for "prlctl register" def register(pvm_file, opts=[]) - args = [@prlctl_path, 'register', pvm_file, *opts] - - 3.times do - result = raw(*args) - # Exit if everything is OK - return if result.exit_code == 0 - - # It may occur in the race condition with other Vagrant processes. - # It is OK, just exit. - return if result.stderr.include?('is already registered.') - - # Sleep a bit though to give Parallels Desktop time to fix itself - sleep 2 - end - - # If we reach this point, it means that we consistently got the - # failure, do a standard execute now. This will raise an - # exception if it fails again. - execute(*args) + execute_prlctl('register', pvm_file, *opts) end # Switches the VM state to the specified snapshot @@ -622,25 +604,7 @@ def suspend # Virtual machine will be removed from the VM list, but its image will # not be deleted from the disk. So, it can be registered again. def unregister(uuid) - args = [@prlctl_path, 'unregister', uuid] - 3.times do - result = raw(*args) - # Exit if everything is OK - return if result.exit_code == 0 - - # It may occur in the race condition with other Vagrant processes. - # Both are OK, just exit. - return if result.stderr.include?('is not registered') - return if result.stderr.include?('is being cloned') - - # Sleep a bit though to give Parallels Desktop time to fix itself - sleep 2 - end - - # If we reach this point, it means that we consistently got the - # failure, do a standard execute now. This will raise an - # exception if it fails again. - execute(*args) + execute_prlctl('unregister', uuid) end # Unshare folders. From 206ba425775ed93c7ca3d7efea45134971ba116d Mon Sep 17 00:00:00 2001 From: Mikhail Zholobov Date: Wed, 13 Jan 2016 16:30:18 +0200 Subject: [PATCH 3/4] Call fsync and flash before closing the box lease file It guarantees that counter is written on the disk and IO buffer is flushed. --- lib/vagrant-parallels/action/box_register.rb | 2 ++ lib/vagrant-parallels/action/box_unregister.rb | 2 ++ 2 files changed, 4 insertions(+) diff --git a/lib/vagrant-parallels/action/box_register.rb b/lib/vagrant-parallels/action/box_register.rb index c8a57d19..2927b297 100644 --- a/lib/vagrant-parallels/action/box_register.rb +++ b/lib/vagrant-parallels/action/box_register.rb @@ -77,6 +77,8 @@ def lease_box_lock(env) num = file.gets.to_i file.rewind file.puts num.next + file.fsync + file.flush end end diff --git a/lib/vagrant-parallels/action/box_unregister.rb b/lib/vagrant-parallels/action/box_unregister.rb index 17dc0854..ec80b411 100644 --- a/lib/vagrant-parallels/action/box_unregister.rb +++ b/lib/vagrant-parallels/action/box_unregister.rb @@ -39,6 +39,8 @@ def release_box_lock(lease_file) num = file.gets.to_i file.rewind file.puts(num - 1) + file.fsync + file.flush end # Delete the lease file if we are the last who need this box. From e89b0aff6c3ea398a0c337da433a1dc089843301 Mon Sep 17 00:00:00 2001 From: Mikhail Zholobov Date: Wed, 13 Jan 2016 16:31:15 +0200 Subject: [PATCH 4/4] action/unregister: Add parallel-safe lock to prevent concurrent writes to box lease file --- lib/vagrant-parallels/action/box_unregister.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/vagrant-parallels/action/box_unregister.rb b/lib/vagrant-parallels/action/box_unregister.rb index ec80b411..fb00e4ee 100644 --- a/lib/vagrant-parallels/action/box_unregister.rb +++ b/lib/vagrant-parallels/action/box_unregister.rb @@ -4,6 +4,8 @@ module VagrantPlugins module Parallels module Action class BoxUnregister + @@lock = Mutex.new + def initialize(app, env) @app = app @logger = Log4r::Logger.new('vagrant_parallels::action::box_unregister') @@ -15,7 +17,12 @@ def call(env) return @app.call(env) end - unregister_box(env) + @@lock.synchronize do + lock_key = Digest::MD5.hexdigest(env[:machine].box.name) + env[:machine].env.lock(lock_key, retry: true) do + unregister_box(env) + end + end # If we got interrupted, then the import could have been # interrupted and its not a big deal. Just return out.