Remove need to pass machine and domain around in driver (#1579)

The driver is created with a specific machine instance, use this in
stead of requiring a specific instance be passed in. Apply the same
approach to domain where possible which allows the same get ip address
function to be called with and without a domain being passed in to save
lookup during loops.

lib/vagrant-libvirt/action/wait_till_up.rb

@@ -23,7 +23,7 @@ module VagrantPlugins
           env[:metrics] ||= {}
 
           driver = env[:machine].provider.driver
-          domain = driver.get_domain(env[:machine])
+          domain = driver.get_domain
 
           if domain.nil?
             raise Errors::NoDomainError,
@@ -42,7 +42,7 @@ module VagrantPlugins
               return if env[:interrupted]
 
               # Wait for domain to obtain an ip address
-              env[:ip_address] = driver.get_domain_ipaddress(env[:machine], domain)
+              env[:ip_address] = driver.get_ipaddress(domain)
             end
           end
           @logger.info("Got IP address #{env[:ip_address]}")

lib/vagrant-libvirt/driver.rb

@@ -66,65 +66,48 @@ module VagrantPlugins
         @system_connection
       end
 
-      def get_domain(machine)
-        begin
-          domain = connection.servers.get(machine.id)
-        rescue Libvirt::RetrieveError => e
-          raise e unless e.libvirt_code == ProviderLibvirt::Util::ErrorCodes::VIR_ERR_NO_DOMAIN
-
-          @logger.debug("machine #{machine.name} domain not found #{e}.")
-          return nil
-        end
-
-        domain
-      end
-
-      def created?(machine)
-        domain = get_domain(machine)
+      def created?
+        domain = get_domain()
         !domain.nil?
       end
 
-      def get_ipaddress(machine)
+      def get_ipaddress(domain=nil)
         # Find the machine
-        domain = get_domain(machine)
+        domain = get_domain() if domain.nil?
 
         if domain.nil?
           # The machine can't be found
           return nil
         end
 
-        get_domain_ipaddress(machine, domain)
-      end
-
-      def get_domain_ipaddress(machine, domain)
         # attempt to get ip address from qemu agent
-        if machine.provider_config.qemu_use_agent == true
+        if @machine.provider_config.qemu_use_agent == true
           @logger.info('Get IP via qemu agent')
-          return get_ipaddress_from_qemu_agent(domain, machine.id, machine.config.vm.boot_timeout)
+          return get_ipaddress_from_qemu_agent(@machine.config.vm.boot_timeout, domain)
         end
 
-        return get_ipaddress_from_system domain.mac if machine.provider_config.qemu_use_session
+        return get_ipaddress_from_system domain.mac if @machine.provider_config.qemu_use_session
 
         # Get IP address from dhcp leases table
         begin
           ip_address = get_ipaddress_from_domain(domain)
         rescue Fog::Errors::TimeoutError
-          @logger.info("Timeout at waiting for an ip address for machine #{machine.name}")
+          @logger.info("Timeout at waiting for an ip address for machine #{@machine.name}")
 
           raise
         end
 
         unless ip_address
-          @logger.info("No arp table entry found for machine #{machine.name}")
+          @logger.info("No arp table entry found for machine #{@machine.name}")
           return nil
         end
 
         ip_address
       end
 
-      def restore_snapshot(machine, snapshot_name)
-        domain = get_libvirt_domain(machine)
-        snapshot = get_snapshot_if_exists(machine, snapshot_name)
+      def restore_snapshot(snapshot_name)
+        domain = get_libvirt_domain()
+        snapshot = get_snapshot_if_exists(@machine, snapshot_name)
         begin
           # 4 is VIR_DOMAIN_SNAPSHOT_REVERT_FORCE
           # needed due to https://bugzilla.redhat.com/show_bug.cgi?id=1006886
@@ -134,52 +117,52 @@ module VagrantPlugins
         end
       end
 
-      def list_snapshots(machine)
-        get_libvirt_domain(machine).list_snapshots
+      def list_snapshots
+        get_libvirt_domain().list_snapshots
       rescue Fog::Errors::Error => e
         raise Errors::SnapshotListError, error_message: e.message
       end
 
-      def delete_snapshot(machine, snapshot_name)
-        get_snapshot_if_exists(machine, snapshot_name).delete
+      def delete_snapshot(snapshot_name)
+        get_snapshot_if_exists(snapshot_name).delete
       rescue Errors::SnapshotMissing => e
         raise Errors::SnapshotDeletionError, error_message: e.message
       end
 
-      def create_new_snapshot(machine, snapshot_name)
+      def create_new_snapshot(snapshot_name)
         snapshot_desc = <<-EOF
         <domainsnapshot>
           <name>#{snapshot_name}</name>
           <description>Snapshot for vagrant sandbox</description>
         </domainsnapshot>
         EOF
-        get_libvirt_domain(machine).snapshot_create_xml(snapshot_desc)
+        get_libvirt_domain().snapshot_create_xml(snapshot_desc)
       rescue Fog::Errors::Error => e
         raise Errors::SnapshotCreationError, error_message: e.message
       end
 
-      def create_snapshot(machine, snapshot_name)
+      def create_snapshot(snapshot_name)
         begin
-          delete_snapshot(machine, snapshot_name)
+          delete_snapshot(snapshot_name)
         rescue Errors::SnapshotDeletionError
         end
-        create_new_snapshot(machine, snapshot_name)
+        create_new_snapshot(snapshot_name)
       end
 
       # if we can get snapshot description without exception it exists
-      def get_snapshot_if_exists(machine, snapshot_name)
-        snapshot = get_libvirt_domain(machine).lookup_snapshot_by_name(snapshot_name)
+      def get_snapshot_if_exists(snapshot_name)
+        snapshot = get_libvirt_domain().lookup_snapshot_by_name(snapshot_name)
         return snapshot if snapshot.xml_desc
       rescue Libvirt::RetrieveError => e
         raise Errors::SnapshotMissing, error_message: e.message
       end
 
-      def state(machine)
+      def state
         # may be other error states with initial retreival we can't handle
         begin
-          domain = get_domain(machine)
+          domain = get_domain
         rescue Libvirt::RetrieveError => e
-          @logger.debug("Machine #{machine.id} not found #{e}.")
+          @logger.debug("Machine #{@machine.id} not found #{e}.")
           return :not_created
         end
 
@@ -191,9 +174,9 @@ module VagrantPlugins
         state = domain.state.tr('-', '_').to_sym
         if state == :running
           begin
-            get_domain_ipaddress(machine, domain)
+            get_ipaddress(domain)
           rescue Fog::Errors::TimeoutError => e
-            @logger.debug("Machine #{machine.id} running but no IP address available: #{e}.")
+            @logger.debug("Machine #{@machine.id} running but no IP address available: #{e}.")
             return :inaccessible
           end
         end
@@ -201,6 +184,19 @@ module VagrantPlugins
         state
       end
 
+      def get_domain
+        begin
+          domain = connection.servers.get(@machine.id)
+        rescue Libvirt::RetrieveError => e
+          raise e unless e.libvirt_code == ProviderLibvirt::Util::ErrorCodes::VIR_ERR_NO_DOMAIN
+
+          @logger.debug("machine #{@machine.name} domain not found #{e}.")
+          return nil
+        end
+
+        domain
+      end
+
       def list_host_devices
         connection.client.list_all_interfaces
       end
@@ -209,6 +205,14 @@ module VagrantPlugins
         connection.client.list_all_networks
       end
 
+      def attach_device(xml)
+        get_libvirt_domain.attach_device(xml)
+      end
+
+      def detach_device(xml)
+        get_libvirt_domain.detach_device(xml)
+      end
+
       private
 
       def get_ipaddress_from_system(mac)
@@ -224,10 +228,10 @@ module VagrantPlugins
         ip_address
       end
 
-      def get_ipaddress_from_qemu_agent(domain, machine_id, timeout)
+      def get_ipaddress_from_qemu_agent(timeout, domain=nil)
         ip_address = nil
         addresses = nil
-        libvirt_domain = connection.client.lookup_domain_by_uuid(machine_id)
+        libvirt_domain = get_libvirt_domain()
         begin
           response = libvirt_domain.qemu_agent_command('{"execute":"guest-network-get-interfaces"}', timeout)
           @logger.debug('Got Response from qemu agent')
@@ -274,13 +278,13 @@ module VagrantPlugins
         ip_address
       end
 
-      def get_libvirt_domain(machine)
+      def get_libvirt_domain
         begin
-          libvirt_domain = connection.client.lookup_domain_by_uuid(machine.id)
+          libvirt_domain = connection.client.lookup_domain_by_uuid(@machine.id)
         rescue Libvirt::RetrieveError => e
           raise e unless e.libvirt_code == ProviderLibvirt::Util::ErrorCodes::VIR_ERR_NO_DOMAIN
 
-          @logger.debug("machine #{machine.name} not found #{e}.")
+          @logger.debug("machine #{@machine.name} not found #{e}.")
           return nil
         end
 

lib/vagrant-libvirt/provider.rb

@@ -57,7 +57,7 @@ module VagrantPlugins
         # be called from other threads of execution.
         return nil if state.id != :running
 
-        ip = driver.get_ipaddress(@machine)
+        ip = driver.get_ipaddress
 
         # if can't determine the IP, just return nil and let the core
         # deal with it, similar to the docker provider
@@ -93,9 +93,9 @@ module VagrantPlugins
         state_id = nil
         state_id = :not_created unless @machine.id
         state_id = :not_created if
-          !state_id && (!@machine.id || !driver.created?(@machine))
+          !state_id && (!@machine.id || !driver.created?)
         # Query the driver for the current state of the machine
-        state_id = driver.state(@machine) if @machine.id && !state_id
+        state_id = driver.state if @machine.id && !state_id
         state_id = :unknown unless state_id
 
         # This is a special pseudo-state so that we don't set the

spec/unit/action/wait_till_up_spec.rb

@@ -74,7 +74,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::WaitTillUp do
         allow(domain).to receive(:wait_for).and_return(true)
         allow(env).to receive(:[]).and_call_original
         allow(env).to receive(:[]).with(:interrupted).and_return(false)
-        allow(driver).to receive(:get_domain_ipaddress).and_return('192.168.121.2')
+        allow(driver).to receive(:get_ipaddress).and_return('192.168.121.2')
       end
       it 'should call the next hook' do
         expect(app).to receive(:call)

spec/unit/driver_spec.rb

@@ -114,7 +114,7 @@ describe VagrantPlugins::ProviderLibvirt::Driver do
         # ideally should be able to yield a block to wait_for and check that
         # the 'addresses' function on the domain is called correctly.
         expect(domain).to receive(:wait_for).and_return(nil)
-        expect(subject.get_ipaddress(machine)).to eq(nil)
+        expect(subject.get_ipaddress).to eq(nil)
       end
 
       context 'when qemu_use_agent is enabled' do
@@ -159,7 +159,7 @@ describe VagrantPlugins::ProviderLibvirt::Driver do
           expect(libvirt_domain).to receive(:qemu_agent_command).and_return(qemu_agent_interfaces)
           expect(domain).to receive(:mac).and_return("52:54:00:f8:67:98").exactly(2).times
 
-          expect(subject.get_ipaddress(machine)).to eq("192.168.122.42")
+          expect(subject.get_ipaddress).to eq("192.168.122.42")
         end
 
         context 'when qemu_use_session is enabled' do
@@ -173,7 +173,7 @@ describe VagrantPlugins::ProviderLibvirt::Driver do
             expect(libvirt_domain).to receive(:qemu_agent_command).and_return(qemu_agent_interfaces)
             expect(domain).to receive(:mac).and_return("52:54:00:f8:67:98").exactly(2).times
 
-            expect(subject.get_ipaddress(machine)).to eq("192.168.122.42")
+            expect(subject.get_ipaddress).to eq("192.168.122.42")
           end
         end
       end
@@ -203,7 +203,7 @@ describe VagrantPlugins::ProviderLibvirt::Driver do
           expect(system_connection1).to receive(:list_all_networks).and_return(networks)
           expect(networks[0]).to receive(:dhcp_leases).and_return([dhcp_leases])
 
-          expect(subject.get_ipaddress(machine)).to eq("192.168.122.43")
+          expect(subject.get_ipaddress).to eq("192.168.122.43")
         end
 
         context 'when qemu_use_agent is enabled' do
@@ -214,7 +214,7 @@ describe VagrantPlugins::ProviderLibvirt::Driver do
           it 'should retrieve the address via the agent' do
             expect(subject).to receive(:get_ipaddress_from_qemu_agent).and_return("192.168.122.44")
 
-            expect(subject.get_ipaddress(machine)).to eq("192.168.122.44")
+            expect(subject.get_ipaddress).to eq("192.168.122.44")
           end
         end
       end
@@ -271,7 +271,7 @@ describe VagrantPlugins::ProviderLibvirt::Driver do
         {
           :setup => ProcWithBinding.new do
             expect(domain).to receive(:state).and_return('running').at_least(:once)
-            expect(subject).to receive(:get_domain_ipaddress).and_raise(Fog::Errors::TimeoutError)
+            expect(subject).to receive(:get_ipaddress).and_raise(Fog::Errors::TimeoutError)
           end,
         }
       ],
@@ -281,7 +281,7 @@ describe VagrantPlugins::ProviderLibvirt::Driver do
         {
           :setup => ProcWithBinding.new do
             expect(domain).to receive(:state).and_return('running').at_least(:once)
-            expect(subject).to receive(:get_domain_ipaddress).and_return('192.168.121.2')
+            expect(subject).to receive(:get_ipaddress).and_return('192.168.121.2')
           end,
         }
       ],
@@ -294,7 +294,7 @@ describe VagrantPlugins::ProviderLibvirt::Driver do
           opts[:setup].apply_binding(binding)
         end
 
-        expect(subject.state(machine)).to eq(expected)
+        expect(subject.state).to eq(expected)
       end
     end
   end