Reduce nested rescues in start domain (#1573)

Remove the unnecessary nesting of begin/rescue entries in start domain
which partially existed due to vagrant-libvirt destroying domain
definitions on error, even for machines that had previously been
created.

Since this is no longer a problem as existing domains will be halted
rather than removed when an exception occurs, it is perfectly fine to
let the exception percolate upwards and be handled by the warden, which
is what the current code is doing, but at the expense of catching and
re-raising multiple times.

lib/vagrant-libvirt/action/start_domain.rb

@@ -20,11 +20,10 @@ module VagrantPlugins
         def call(env)
           domain = env[:machine].provider.driver.connection.servers.get(env[:machine].id.to_s)
           raise Errors::NoDomainError if domain.nil?
+
           config = env[:machine].provider_config
 
-          begin
           # update domain settings on change.
-
           libvirt_domain = env[:machine].provider.driver.connection.client.lookup_domain_by_uuid(env[:machine].id)
 
           # Libvirt API doesn't support modifying memory on NUMA enabled CPUs
@@ -36,7 +35,6 @@ module VagrantPlugins
             end
           end
 
-            begin
           # XML definition manipulation
           descr = libvirt_domain.xml_desc(1)
           xml_descr = REXML::Document.new descr
@@ -425,10 +423,10 @@ module VagrantPlugins
               # providing XML for the same name and UUID will update the existing domain
               libvirt_domain = env[:machine].provider.driver.connection.define_domain(new_xml)
             rescue ::Libvirt::Error => e
+              env[:ui].error("Error when updating domain settings: #{e.message}")
               raise Errors::UpdateServerError, error_message: e.message
             end
 
-                begin
             # need to check whether the updated XML contains all the changes requested
             proposed = VagrantPlugins::ProviderLibvirt::Util::Xml.new(new_xml)
             applied = VagrantPlugins::ProviderLibvirt::Util::Xml.new(libvirt_domain.xml_desc(1))
@@ -442,17 +440,18 @@ module VagrantPlugins
                 "--- requested\n+++ result\n#{diff}\n" +
                 "Typically this means there is a bug in the XML being sent, please log an issue"
 
+              env[:ui].error("Updated domain settings did not fully apply, attempting restore to previous definition: #{error_msg}")
+              begin
+                env[:machine].provider.driver.connection.define_domain(descr)
+              rescue Fog::Errors::Error => e
+                env[:ui].error("Failed to restore previous domain definition: #{e.message}")
+              end
+
               raise Errors::UpdateServerError, error_message: error_msg
             end
-                rescue Exception => e
-                  env[:machine].provider.driver.connection.define_domain(descr)
-                  raise
-                end
-              end
-            rescue Errors::VagrantLibvirtError => e
-              env[:ui].error("Error when updating domain settings: #{e.message}")
-              raise
           end
+
+          begin
             # Autostart with host if enabled in Vagrantfile
             libvirt_domain.autostart = config.autostart
             @logger.debug {
@@ -462,7 +461,7 @@ module VagrantPlugins
             env[:ui].info(I18n.t('vagrant_libvirt.starting_domain'))
             domain.start
           rescue Fog::Errors::Error, Errors::VagrantLibvirtError => e
-            raise Errors::FogError, message: e.message
+            raise Errors::DomainStartError, error_message: e.message
           end
 
           @app.call(env)

lib/vagrant-libvirt/errors.rb

@@ -178,6 +178,10 @@ module VagrantPlugins
         error_key(:no_domain_error)
       end
 
+      class DomainStartError < VagrantLibvirtError
+        error_key(:domain_start_error)
+      end
+
       class AttachDeviceError < VagrantLibvirtError
         error_key(:attach_device_error)
       end

locales/en.yml

@@ -153,6 +153,8 @@ en:
         Error while downloading volume '%{volume_name}' from storage pool '%{pool_name}': %{error_message}
       no_domain_error: |-
         No domain found. %{error_message}
+      domain_start_error: |-
+        Failed to start domain: %{error_message}
       attach_device_error: |-
         Error while attaching new device to domain. %{error_message}
       detach_device_error: |-

spec/unit/action/start_domain_spec.rb

@@ -94,7 +94,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::StartDomain do
         expect(libvirt_domain).to receive(:xml_desc).and_return(domain_xml, updated_domain_xml)
         expect(domain).to_not receive(:start)
 
-        expect { subject.call(env) }.to raise_error(VagrantPlugins::ProviderLibvirt::Errors::FogError)
+        expect { subject.call(env) }.to raise_error(VagrantPlugins::ProviderLibvirt::Errors::UpdateServerError)
       end
     end