From 5fd0f9a59ad1bfbb772157d2a5665974c216e202 Mon Sep 17 00:00:00 2001
From: Alejandro Huertas Herrero <ahuertas@opennebula.io>
Date: Tue, 1 Mar 2022 17:04:20 +0100
Subject: [PATCH] B #5409: avoid clone disk if there are snapshots (#1822)

(cherry picked from commit 73ffded36cfd60db57dc51bcee5f00e0e39520ae)
---
 src/tm_mad/vcenter/clone                      | 28 +++++++++++++++----
 src/tm_mad/vcenter/delete                     |  4 +--
 .../lib/vcenter_driver/virtual_machine.rb     |  4 +--
 3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/src/tm_mad/vcenter/clone b/src/tm_mad/vcenter/clone
index 3de6a6a3c6..e83a953957 100755
--- a/src/tm_mad/vcenter/clone
+++ b/src/tm_mad/vcenter/clone
@@ -104,7 +104,8 @@ target_path_escaped = VCenterDriver::FileHelper
 target_path = VCenterDriver::FileHelper.unescape_path(target_path_escaped)
 
 begin
-    retries ||= 0
+    op_retry = true
+
     vi_client = VCenterDriver::VIClient.new_from_host(host_id)
     disk_xpath ="TEMPLATE/DISK[SOURCE=\"#{src_path_escaped}\"]"
     one_disk = one_vm.retrieve_xmlelements(disk_xpath) rescue nil
@@ -113,6 +114,18 @@ begin
 
     raise 'Cannot find disk element in vm template' unless one_disk
 
+    vm = VCenterDriver::VirtualMachine.new(vi_client, nil, vm_id)
+
+    if vm.snapshots? || vm.one_snapshots?
+        # If there are snapshots we shouldn't retry
+        op_retry = false
+
+        raise 'Existing sytem snapshots, cannot change disks. ' \
+              'Please remove all snapshots and try again.'
+    end
+
+    retries ||= 0
+
     disk = VCenterDriver::VirtualMachine::Disk.one_disk(disk_id, one_disk)
     new_size = disk.new_size
 
@@ -121,7 +134,6 @@ begin
 
     if !(!disk.managed? || (is_storage_drs && disk.volatile?))
         if is_storage_drs
-            vm = VCenterDriver::VirtualMachine.new(vi_client, nil, vm_id)
             # Get a single DS using StoragePod recommendation
             # We overwrite the reference in target_ds_ref from the StoragePod
             # to the single recommended DS
@@ -146,14 +158,20 @@ begin
 rescue StandardError => e
     message = "Error clone virtual disk #{src_path} in "\
               "datastore #{target_ds_name_vc}. "\
-              "Reason: #{e.message}."
+              "Reason: #{e.message}." \
+              "on attempt \##{retries}."
+
+    if op_retry && (retries += 1) > CONFIG[:retries]
+        message << "\nPlease check vCenter logs directly\n"
+    end
+
     OpenNebula.log_error(message)
     if VCenterDriver::CONFIG[:debug_information]
         STDERR.puts "#{message} #{e.backtrace}"
     end
 
-    sleep 60
-    retry if retries < CONFIG[:retries]
+    sleep 60 if op_retry
+    retry if op_retry && retries < CONFIG[:retries]
 
     exit(-1)
 ensure
diff --git a/src/tm_mad/vcenter/delete b/src/tm_mad/vcenter/delete
index 06829dbb13..dbd6204847 100755
--- a/src/tm_mad/vcenter/delete
+++ b/src/tm_mad/vcenter/delete
@@ -102,7 +102,7 @@ begin
     is_disk = path.match(/disk\.\d+$/)
     if is_disk
         dsid = img_path.split('/')[-3] # get dsid from path
-        @error_message = "error deleting disk with #{img_path} "\
+        @error_message = "Error deleting disk with #{img_path} "\
                          "in datastore #{dsid}"
 
         # Get disk info, destroy it if possible
@@ -122,7 +122,7 @@ begin
     end
 rescue StandardError => e
     vi_client.close_connection if vi_client
-    message = "#{@error_message}. Reason: #{e.message}."
+    message = "#{@error_message} Reason: #{e.message}."
     OpenNebula.log_error(message)
     if VCenterDriver::CONFIG[:debug_information]
         STDERR.puts "#{message} #{e.backtrace}"
diff --git a/src/vmm_mad/remotes/lib/vcenter_driver/virtual_machine.rb b/src/vmm_mad/remotes/lib/vcenter_driver/virtual_machine.rb
index b687b9c449..e737937bde 100644
--- a/src/vmm_mad/remotes/lib/vcenter_driver/virtual_machine.rb
+++ b/src/vmm_mad/remotes/lib/vcenter_driver/virtual_machine.rb
@@ -2438,7 +2438,7 @@ end
             # https://github.com/OpenNebula/one/issues/5409
             if snapshots? || one_snapshots?
                 error_msg =  'Existing sytem snapshots, cannot change disks. '
-                error_msg << 'Please remove all snapshots and try again.'
+                error_msg << 'Please remove all snapshots and try again'
                 raise error_msg
             end
 
@@ -2625,7 +2625,7 @@ end
 
             if snapshots? || one_snapshots?
                 error_message =  'Existing sytem snapshots, cannot change disks'
-                error_message << '. Please remove all snapshots and try again.'
+                error_message << '. Please remove all snapshots and try again'
                 raise error_message
             end