CA-41230: In VM.{snapshot,clone}, keep track of whether a new VDI has actually been created and therefore, whether that VDI should be deleted on failure. In particular CDs are shared not duplicated and so the cleanup code shouldn't try to delete them.
CA-41230: In VM.{snapshot,clone}, keep track of whether a new VDI has actually been created and therefore, whether that VDI should be deleted on failure. In particular CDs are shared not duplicated and so the cleanup code shouldn't try to delete them.

Unfortunately the 'writable ISO SR' support changes the default NFS ISO SR mount options to read/write from read/only, exposing this bug.

Signed-off-by: David Scott <dave.scott@eu.citrix.com>

diff -r 2a39d3f3842e ocaml/xapi/xapi_vm_clone.ml--- a/ocaml/xapi/xapi_vm_clone.ml	Mon May 10 17:14:10 2010 +0100
+++ b/ocaml/xapi/xapi_vm_clone.ml	Mon May 10 17:14:17 2010 +0100
@@ -21,7 +21,11 @@
 open D

 let delete_disks rpc session_id disks =
-	List.iter (fun (vbd,vdi) -> try Client.VDI.destroy rpc session_id vdi with _ -> ()) disks
+	List.iter (fun (vbd,vdi,on_error_delete) ->
+				   if on_error_delete
+				   then try Client.VDI.destroy rpc session_id vdi with _ -> ()
+				   else debug "Not destroying CD VDI: %s" (Ref.string_of vdi)
+			  ) disks

 let wait_for_clone ?progress_minmax ~__context task =
 	Helpers.call_api_functions ~__context (fun rpc session ->
@@ -134,14 +138,14 @@
 			(* If the VBD is empty there is no VDI to copy. *)
 			(* If the VBD is a CD then eject it (we cannot make copies of ISOs: they're identified *)
 			(* by their filename unlike other VDIs) *)
-			let newvdi =
+			let newvdi, on_error_delete =
 				if vbd_r.API.vBD_empty
-				then Ref.null
+				then Ref.null, false
 				else if vbd_r.API.vBD_type = `CD
-				then vbd_r.API.vBD_VDI
-				else clone_single_vdi ~progress:(done_so_far, size, total) rpc session_id disk_op ~__context vbd_r.API.vBD_VDI driver_params
+				then vbd_r.API.vBD_VDI, false (* don't delete the original CD *)
+				else clone_single_vdi ~progress:(done_so_far, size, total) rpc session_id disk_op ~__context vbd_r.API.vBD_VDI driver_params, true (* do delete newly created VDI *)
 			in
-			((vbd,newvdi)::acc, (Int64.add done_so_far size))
+			((vbd,newvdi,on_error_delete)::acc, (Int64.add done_so_far size))
 		with e ->
 			debug "Error in safe_clone_disks: %s" (Printexc.to_string e);
 			delete_disks rpc session_id acc; (* Delete those cloned so far *)
@@ -348,7 +352,7 @@

 				(* copy VBDs *)
 				let new_vbds : [`VBD] Ref.t list =
-					List.map (fun (vbd, newvdi) -> Xapi_vbd_helpers.copy ~__context ~vm:ref ~vdi:newvdi vbd) cloned_disks in
+					List.map (fun (vbd, newvdi, _) -> Xapi_vbd_helpers.copy ~__context ~vm:ref ~vdi:newvdi vbd) cloned_disks in

 				(* copy VIFs *)
 				let new_vifs : [`VIF] Ref.t list =

robhoes/squeezed,koushikcgit/xcp-rrdd,djs55/squeezed,djs55/xcp-rrdd,simonjbeaumont/xcp-rrdd,simonjbeaumont/xcp-rrdd,djs55/xcp-rrdd,koushikcgit/xcp-rrdd,johnelse/xcp-rrdd,johnelse/xcp-rrdd,djs55/xcp-networkd,koushikcgit/xcp-rrdd,sharady/xcp-networkd,koushikcgit/xcp-networkd,sharady/xcp-networkd,koushikcgit/xcp-networkd,djs55/xcp-networkd