From 222b50925d967facab4a44bc4854b5f3322161e2 Mon Sep 17 00:00:00 2001
From: Martin Schwenke <martin@meltin.net>
Date: Wed, 13 Mar 2019 20:49:11 +1100
Subject: [PATCH] Avoid TOCTOU error in volume creation

Checking if a volume exists before attempting to create it results in
a "time of check to time of use" race.  When the check is done the
volume doesn't exist but then, because it is shared storage, it is
then created by another node before the local attempt to create it.
This results in an unexpected failure.

It is better to simply attempt to create the volume and ignore EEXIST
in cases where this is permissible.

Implementing this properly is complicated by the fact that the
exception appears to contain only an error message and does not appear
to contain a useful error code.  Digging through the layers in, for
example, fog-libvirt provides no evidence why this is so.  However,
the error message seems fairly unique and matching it is easy, so just
do that.  A small hack to vastly improve stability.

Signed-off-by: Martin Schwenke <martin@meltin.net>
---
 lib/vagrant-libvirt/action/create_domain.rb | 36 +++++++++++----------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/lib/vagrant-libvirt/action/create_domain.rb b/lib/vagrant-libvirt/action/create_domain.rb
index 1862237..ba5c8d2 100644
--- a/lib/vagrant-libvirt/action/create_domain.rb
+++ b/lib/vagrant-libvirt/action/create_domain.rb
@@ -162,26 +162,28 @@ module VagrantPlugins
 
             disk[:absolute_path] = storage_prefix + disk[:path]
 
-            if env[:machine].provider.driver.connection.volumes.select do |x|
-              x.name == disk[:name] && x.pool_name == @storage_pool_name
-            end.empty?
-              # make the disk. equivalent to:
-              # qemu-img create -f qcow2 <path> 5g
-              begin
-                env[:machine].provider.driver.connection.volumes.create(
-                  name: disk[:name],
-                  format_type: disk[:type],
-                  path: disk[:absolute_path],
-                  capacity: disk[:size],
-                  #:allocation => ?,
-                  pool_name: @storage_pool_name
-                )
-              rescue Fog::Errors::Error => e
+            # make the disk. equivalent to:
+            # qemu-img create -f qcow2 <path> 5g
+            begin
+              env[:machine].provider.driver.connection.volumes.create(
+                name: disk[:name],
+                format_type: disk[:type],
+                path: disk[:absolute_path],
+                capacity: disk[:size],
+                #:allocation => ?,
+                pool_name: @storage_pool_name
+              )
+            rescue Libvirt::Error => e
+              # It is hard to believe that e contains just a string
+              # and no useful error code!
+              msg = "Call to virStorageVolCreateXML failed: " +
+                    "storage volume '#{disk[:path]}' exists already"
+              if e.message == msg and disk[:allow_existing]
+                disk[:preexisting] = true
+              else
                 raise Errors::FogDomainVolumeCreateError,
                       error_message: e.message
               end
-            else
-              disk[:preexisting] = true
             end
           end