The recent addition of the VDI on_boot and allow_caching parameters affected the...

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 a7d38429fe0b50315b8f266c1c66c16132e28e5f..9611484b9449bbd6bbaf8a981da1b696712235c3 100644 (file)
--- 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 =