add support for uploading an existing disk image

lib/vagrant-libvirt/action/create_domain_disks.rb

@@ -24,29 +24,31 @@ module VagrantPlugins
           disks = env[:disks] || []
 
           disks.each do |disk|
-            # 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],
-                owner: storage_uid(env),
-                group: storage_gid(env),
-                #:allocation => ?,
-                pool_name: disk[:pool],
-              )
-            rescue Libvirt::Error => e
-              # It is hard to believe that e contains just a string
-              # and no useful error code!
-              msgs = [disk[:name], disk[:absolute_path]].map do |name|
-                "Call to virStorageVolCreateXML failed: " +
-                "storage volume '#{name}' exists already"
+            # Don't continue if image already exists in storage pool.
+            volume = env[:machine].provider.driver.connection.volumes.all(
+              name: disk[:name]
+            ).first
+            if volume and volume.id
+              disk[:preexisting] = true
+            elsif disk[:path]
+              @@lock.synchronize do
+                storage_send_box_image(env, config, disk[:path], disk)
+                disk[:uploaded] = true
               end
-              if msgs.include?(e.message) and disk[:allow_existing]
-                disk[:preexisting] = true
-              else
+            else
+              # make the disk. equivalent to:
+              # qemu-img create -f qcow2 <path> 5g
+              begin
+                env[:machine].provider.driver.connection.volumes.create(
+                  :name        => disk[:name],
+                  :pool_name   => disk[:pool],
+                  :format_type => disk[:type],
+                  :capacity    => disk[:size],
+                  :owner       => storage_uid(env),
+                  :group       => storage_gid(env),
+                  # :allocation  => ?,
+                )
+              rescue Libvirt::Error => e
                 raise Errors::FogCreateDomainVolumeError,
                       error_message: e.message
               end

lib/vagrant-libvirt/action/resolve_disk_settings.rb

@@ -5,6 +5,7 @@ require 'log4r'
 require 'rexml/document'
 require 'rexml/xpath'
 
+require 'vagrant-libvirt/util/byte_number'
 require 'vagrant-libvirt/util/resolvers'
 
 module VagrantPlugins
@@ -117,15 +118,30 @@ module VagrantPlugins
           resolver.resolve!(disks)
 
           disks.each do |disk|
-            disk[:path] ||= disk_name(domain_name, disk)
+            # The original version of this plugin only exposed the :path param,
+            # but libvirt only cares about the <name> parameter because volumes
+            # are assigned a storage pool which defines the base directory.
+            # To work around this, :name used to be blindly set to :path, and
+            # :path did not support absolute paths.
+            # The new behavior preserves the old behavior, while introducing
+            # the option to set :path to an existing disk image that will be
+            # uploaded to the specified storage pool.
+            if disk[:path] and File.exists?(disk[:path])
+              disk[:name] = File.basename(disk[:path])
+              disk[:path] = File.absolute_path(disk[:path])
+              disk[:size] = ByteNumber.new(File.size(disk[:path]))
+              disk[:virtual_size] = ByteNumber.new(File.size(disk[:path]))
+            else
+              if disk[:path]
+                disk[:name] = disk[:path]
+                disk.delete(:path)
+              else
+                disk[:name] = disk_name(domain_name, disk)
+                disk.delete(:path)
+              end
+            end
 
-            # On volume creation, the <path> element inside <target>
-            # is oddly ignored; instead the path is taken from the
-            # <name> element:
-            # http://www.redhat.com/archives/libvir-list/2008-August/msg00329.html
-            disk[:name] = disk[:path]
-
-            disk[:absolute_path] = storage_prefix + disk[:path]
+            disk[:absolute_path] = storage_prefix + disk[:name]
 
             if disk[:pool].nil?
               disk[:pool] = storage_pool_name

lib/vagrant-libvirt/config.rb

@@ -816,6 +816,7 @@ module VagrantPlugins
           type: options[:type],
           address_type: options[:address_type],
           size: options[:size],
+          name: options[:name],
           path: options[:path],
           bus: options[:bus],
           cache: options[:cache] || 'default',
@@ -1209,8 +1210,11 @@ module VagrantPlugins
         end
 
         machine.provider_config.disks.each do |disk|
-          if disk[:path] && (disk[:path][0] == '/')
-            errors << "absolute volume paths like '#{disk[:path]}' not yet supported"
+          if disk[:name] and File.exists?(disk[:name])
+            errors << ":name does not accept file paths like '#{disk[:name]}'"
+          end
+          if disk[:path] and !File.exists?(disk[:path])
+            errors << ":path does not exist: '#{disk[:path]}'"
           end
         end
 

spec/unit/action/create_domain_disks_spec.rb

@@ -26,45 +26,83 @@ describe VagrantPlugins::ProviderLibvirt::Action::CreateDomainDisks do
     end
 
     context 'additional disks' do
-      let(:vagrantfile_providerconfig) do
-        <<-EOF
-        libvirt.qemu_use_session = true
-        EOF
-      end
 
       let(:disks) do
         [
-          :device        => 'vdb',
-          :cache         => 'default',
-          :bus           => 'virtio',
-          :type          => 'qcow2',
-          :absolute_path => '/var/lib/libvirt/images/vagrant-test_default-vdb.qcow2',
-          :virtual_size  => ByteNumber.new(20*1024*1024*1024),
-          :pool          => 'default',
+          {
+            :device        => 'vdb',
+            :cache         => 'default',
+            :bus           => 'virtio',
+            :type          => 'qcow2',
+            :name          => 'vagrant-test_default-vdb.qcow2',
+            :absolute_path => '/var/lib/libvirt/images/vagrant-test_default-vdb.qcow2',
+            :virtual_size  => ByteNumber.new(20*1024*1024*1024),
+            :pool          => 'default',
+          },
         ]
       end
 
       before do
-        expect(Process).to receive(:uid).and_return(9999).at_least(:once)
-        expect(Process).to receive(:gid).and_return(9999).at_least(:once)
-
         env[:disks] = disks
       end
 
-      context 'volume create succeeded' do
-        it 'should complete' do
-          expect(volumes).to receive(:create).with(
-            hash_including(
-              :path        => "/var/lib/libvirt/images/vagrant-test_default-vdb.qcow2",
-              :owner       => 9999,
-              :group       => 9999,
-              :pool_name   => "default",
-            )
-          )
+      context 'volume already exists' do
+        let(:volume) { instance_double(::Fog::Libvirt::Compute::Volume) }
+
+        before do
+          allow(volumes).to receive(:all).and_return([volume])
+          allow(volume).to receive(:id).and_return(1)
+        end
+
+        it 'should succeed and set :preexisting' do
+          expect(subject.call(env)).to be_nil
+          expect(disks[0][:preexisting]).to be(true)
+        end
+      end
+
+      context 'volume needs uploading' do
+        let(:tmp_fh) { Tempfile.new('vagrant-libvirt') }
+
+        before do
+          env[:disks][0][:path] = tmp_fh.path
+          allow(volumes).to receive(:all).and_return([])
+        end
+
+        after do
+          tmp_fh.delete
+        end
+
+        it 'should upload and succeed' do
+          expect(subject).to receive(:storage_upload_image).and_return(true)
+
+          expect(subject.call(env)).to be_nil
+          expect(disks[0][:uploaded]).to be(true)
+        end
+      end
+
+      context 'volume must be created' do
+
+        before do
+          allow(volumes).to receive(:all).and_return([])
+        end
+
+        it 'should succeed' do
+          expect(disks[0][:path]).to be_nil
+          expect(volumes).to receive(:create).and_return(nil)
 
           expect(subject.call(env)).to be_nil
         end
+
+        it 'should fail' do
+          expect(disks[0][:path]).to be_nil
+          expect(volumes).to receive(:create).and_raise(Libvirt::Error)
+
+          expect{ subject.call(env) }.to raise_error(
+            VagrantPlugins::ProviderLibvirt::Errors::FogCreateDomainVolumeError
+          )
+        end
       end
+
     end
   end
 end

spec/unit/config_spec.rb

@@ -757,15 +757,32 @@ describe VagrantPlugins::ProviderLibvirt::Config do
     end
 
     context 'with disks defined' do
-      it 'is valid if relative path used for disk' do
-        subject.storage :file, path: '../path/to/file.qcow2'
+      let(:tmp_fh) { Tempfile.new('vagrant-libvirt') }
+
+      after do
+        tmp_fh.delete
+      end
+
+      it 'is valid if :name is just a name' do
+        subject.storage :file, name: 'file.qcow2'
         assert_valid
       end
 
-      it 'should be invalid if absolute path used for disk' do
-        subject.storage :file, path: '/absolute/path/to/file.qcow2'
+      # Leave it to libvirt to error on names that could be paths
+      it 'is valid if :name is a path that does not exist' do
+        subject.storage :file, name: './path/to/file.qcow2'
+        assert_valid
+      end
+
+      it 'is invalid if :path does not exist' do
+        subject.storage :file, path: './path/to/file.qcow2'
         assert_invalid
       end
+
+      it 'is valid if :path exists' do
+        subject.storage :file, path: tmp_fh.path
+        assert_valid
+      end
     end
 
     context 'with mac defined' do