From: Jon Ludlam <jonathan.ludlam@eu.citrix.com>
Date: Mon, 11 Oct 2010 21:57:29 +0000 (+0100)
Subject: The recent addition of the VDI on_boot and allow_caching parameters affected the... 
X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=762676d792987b76383ddb6c91c45e660148179f;p=xcp%2Fxen-api.git

The recent addition of the VDI on_boot and allow_caching parameters affected the allowed operations check. The additional check was put in
the middle of the if block rather than at the end, thus short circuiting some subsequent checks that would otherwise have been done. For
example, checkpoint is allowed in a VM with no tools. This patch fixes that behaviour.

Signed-off-by: Jon Ludlam <jonathan.ludlam@eu.citrix.com>

 diff -r 95b9e4f1b9dd ocaml/xapi/xapi_vm_lifecycle.ml
---

diff --git a/ocaml/xapi/xapi_vm_lifecycle.ml b/ocaml/xapi/xapi_vm_lifecycle.ml
index a7d38429..9611484b 100644
--- a/ocaml/xapi/xapi_vm_lifecycle.ml
+++ b/ocaml/xapi/xapi_vm_lifecycle.ml
@@ -246,18 +246,6 @@ let check_operation_error ~vmr ~vmgmr ~ref ~clone_suspended_vm_enabled vdis_rese
 		&& op <> `changing_dynamic_range
 	then Some (Api_errors.operation_not_allowed, ["Operations on domain 0 are not allowed"])
 
-	(* Check for an error due to VDI caching/reset behaviour *)
-	else if op = `checkpoint || op = `snapshot || op = `suspend || op = `snapshot_with_quiesce
- 	then (* If any vdi exists with on_boot=reset, then disallow checkpoint, snapshot, suspend *)
-		if List.exists fst vdis_reset_and_caching 
-		then Some (Api_errors.vdi_on_boot_mode_incompatable_with_operation,[]) 
-		else None
-	else if op = `pool_migrate then
-		(* If any vdi exists with on_boot=reset and caching is enabled, disallow migrate *)
-		if List.exists (fun (reset,caching) -> reset && caching) vdis_reset_and_caching
-		then Some (Api_errors.vdi_on_boot_mode_incompatable_with_operation,[]) 
-		else None
-
 	(* check PV drivers constraints if needed *)
 	else if need_pv_drivers_check ~power_state ~op
 	then check_drivers ~vmr ~vmgmr ~op ~ref
@@ -273,6 +261,19 @@ let check_operation_error ~vmr ~vmgmr ~ref ~clone_suspended_vm_enabled vdis_rese
 			  not (List.mem_assoc "feature-quiesce" other || List.mem_assoc "feature-snapshot" other)) 
 			 vmgmr)
 	then Some (Api_errors.vm_snapshot_with_quiesce_not_supported, [ ref_str ])
+
+	(* Check for an error due to VDI caching/reset behaviour *)
+	else if op = `checkpoint || op = `snapshot || op = `suspend || op = `snapshot_with_quiesce
+ 	then (* If any vdi exists with on_boot=reset, then disallow checkpoint, snapshot, suspend *)
+		if List.exists fst vdis_reset_and_caching 
+		then Some (Api_errors.vdi_on_boot_mode_incompatable_with_operation,[]) 
+		else None
+	else if op = `pool_migrate then
+		(* If any vdi exists with on_boot=reset and caching is enabled, disallow migrate *)
+		if List.exists (fun (reset,caching) -> reset && caching) vdis_reset_and_caching
+		then Some (Api_errors.vdi_on_boot_mode_incompatable_with_operation,[]) 
+		else None
+
 	else None
 
 let maybe_get_guest_metrics ~__context ~ref =