Select datastore by its association with the provider

Datastores on the same setup may have the same name.
However, there is an assumption that datastores names on the same
provider are unique. The selection of datastore by its name should be
narrow to the provider on which the vm is provisioned.

https://bugzilla.redhat.com/show_bug.cgi?id=1427498

app/models/vm_or_template/operations/configuration.rb

@@ -74,8 +74,10 @@ def disconnect_floppies
   def raw_add_disk(disk_name, disk_size_mb, options = {})
     raise _("VM has no EMS, unable to add disk") unless ext_management_system
     if options[:datastore]
-      datastore = Storage.find_by(:name => options[:datastore])
-      raise _("Data Store does not exist, unable to add disk") unless datastore
+      datastore = ext_management_system.hosts.collect do |h|
+        h.writable_storages.find_by(:name => options[:datastore])
+      end.uniq.compact.first
+      raise _("Datastore does not exist or cannot be accessed, unable to add disk") unless datastore
     end
 
     run_command_via_parent(:vm_add_disk, :diskName => disk_name, :diskSize => disk_size_mb,

spec/models/vm_or_template/operations/configuration_spec.rb

@@ -0,0 +1,59 @@
+describe VmOrTemplate::Operations::Configuration do
+  context "#raw_add_disk" do
+    let(:disk_name) { "abc" }
+    let(:disk_size) { "123" }
+
+    context "when ext_management_system does not exist" do
+      let(:vm) { FactoryGirl.create(:vm_or_template) }
+
+      it "raises an exception when does not find ext_management_system" do
+        message = "VM has no EMS, unable to add disk"
+        expect { vm.raw_add_disk(disk_name, disk_size, {}) }.to raise_error(message)
+      end
+    end
+
+    context "when ext_management_system exists" do
+      let(:vm) { FactoryGirl.create(:vm_or_template, :ext_management_system => ems) }
+      let(:ems) { FactoryGirl.create(:ext_management_system) }
+      let(:storage_name) { "test_storage" }
+      let(:storage) { FactoryGirl.create(:storage, :name => storage_name) }
+      let(:storages) { double("storages") }
+      let(:host) { double("host", :writable_storages => storages) }
+      let(:hosts) { [host] }
+
+      before do
+        allow(ems).to receive(:hosts).and_return(hosts)
+      end
+
+      context "when storage exists" do
+        it "adds a disk on the storage" do
+          allow(storages).to receive(:find_by).with(:name => storage_name).and_return(storage)
+          allow(ems).to receive(:authentication_status_ok?).and_return(true)
+          allow(ems).to receive(:vm_add_disk)
+
+          expected_options = {
+            :diskName        => disk_name,
+            :diskSize        => disk_size,
+            :thinProvisioned => nil,
+            :dependent       => nil,
+            :persistent      => nil,
+            :bootable        => nil,
+            :datastore       => storage,
+            :interface       => nil
+          }
+          expect(vm).to receive(:run_command_via_parent).with(:vm_add_disk, expected_options).once
+          vm.raw_add_disk(disk_name, disk_size, :datastore => storage_name)
+        end
+      end
+
+      context "when storage does not exist" do
+        it "raises an exception when doesn't find storage by its name" do
+          allow(storages).to receive(:find_by).with(:name => storage_name).and_return(nil)
+
+          message = "Datastore does not exist or cannot be accessed, unable to add disk"
+          expect { vm.raw_add_disk(disk_name, disk_size, :datastore => storage_name) }.to raise_error(message)
+        end
+      end
+    end
+  end
+end