CA-78221: use sm-config instead of other-config

Also remove the ~params field of create/clone/snapshot calls are now we can pass the parameters using sm-config

Author: samoht

ocaml/sm-cli/main.ml

@@ -103,6 +103,13 @@ let _ =
 			if Array.length Sys.argv < 3 then usage_and_exit ();
 			let kvpairs = kvpairs args in
 			let find key = if List.mem_assoc key kvpairs then Some (List.assoc key kvpairs) else None in
+			let params = List.filter_map
+				(fun (k, v) ->
+					let prefix = "params:" in
+					let l = String.length prefix in
+					if String.startswith prefix k
+					then Some (String.sub k l (String.length k - l), v)
+					else None) kvpairs in
 			let vdi_info = {
 				vdi = "";
 				sr = sr;
@@ -118,17 +125,10 @@ let _ =
 				virtual_size = Opt.default 1L (Opt.map Int64.of_string (find "virtual_size"));
 				physical_utilisation = 0L;
 				persistent = true;
-				base_mirror = find "base_mirror";
+				sm_config = params;
 			} in
-			let params = List.filter_map
-				(fun (k, v) ->
-					let prefix = "params:" in
-					let l = String.length prefix in
-					if String.startswith prefix k
-					then Some (String.sub k l (String.length k - l), v)
-					else None) kvpairs in
 
-			let v = Client.VDI.create ~dbg ~sr ~vdi_info ~params in
+			let v = Client.VDI.create ~dbg ~sr ~vdi_info in
 			Printf.printf "%s\n" (string_of_vdi_info v)
 		| [ "vdi-destroy"; sr; vdi ] ->
 			Client.VDI.destroy ~dbg ~sr ~vdi

ocaml/sm-cli/test.ml

@@ -170,7 +170,7 @@ let example_vdi_info =
 	let read_only = false in
 	let virtual_size = Int64.mul 8L mib in
 	let physical_utilisation = 0L in
-	let base_mirror = None in
+	let sm_config = [] in
 	{
 		vdi = "";
 		sr = "";
@@ -186,13 +186,13 @@ let example_vdi_info =
 		virtual_size = virtual_size;
 		physical_utilisation = physical_utilisation;
 		persistent = true;
-		base_mirror = base_mirror;
+		sm_config = sm_config;
 	}
 
 let test_create_destroy sr _ =
 	let vdi_info = example_vdi_info in
 	let vdi_info' = 
-		let vdi_info' = SMClient.VDI.create ~dbg ~sr ~vdi_info ~params:[] in
+		let vdi_info' = SMClient.VDI.create ~dbg ~sr ~vdi_info in
 		vdi_info_assert_equal vdi_info vdi_info';
 		vdi_info'
 	in
@@ -207,7 +207,7 @@ let test_create_destroy sr _ =
 	end
 
 let test_attach_activate url sr _ =
-	let vdi_info = SMClient.VDI.create ~dbg ~sr ~vdi_info:example_vdi_info ~params:[] in
+	let vdi_info = SMClient.VDI.create ~dbg ~sr ~vdi_info:example_vdi_info in
 	let dp = "test_attach_activate" in
 	let _ = SMClient.VDI.attach ~dbg ~sr ~dp ~vdi:vdi_info.vdi ~read_write:true in
 	SMClient.VDI.activate ~dbg ~sr ~dp ~vdi:vdi_info.vdi;
@@ -230,7 +230,7 @@ let test_attach_activate url sr _ =
 	SMClient.VDI.destroy ~dbg ~sr ~vdi:vdi_info.vdi
 
 let test_mirror_1 url sr rurl rsr _ =
-	let vdi_info = SMClient.VDI.create ~dbg ~sr ~vdi_info:example_vdi_info ~params:[] in
+	let vdi_info = SMClient.VDI.create ~dbg ~sr ~vdi_info:example_vdi_info in
 	let dp = "test_attach_activate" in
 	ignore(SMClient.VDI.attach ~dbg ~sr ~dp ~vdi:vdi_info.vdi ~read_write:true);
 	SMClient.VDI.activate ~dbg ~sr ~dp ~vdi:vdi_info.vdi;

ocaml/xapi/storage_access.ml

@@ -84,13 +84,6 @@ module SMAPIv1 = struct
 		let stat_vdi context ~dbg ~sr ~vdi = assert false
 	end
 
-	let base_mirror vdi_rec = (* CA-78221 *)
-		if List.mem_assoc "base_mirror" vdi_rec.API.vDI_other_config then
-			let id = List.assoc "base_mirror" vdi_rec.API.vDI_other_config in
-			Some id
-		else
-			None
-
 	module SR = struct
 		let create context ~dbg ~sr ~device_config ~physical_size =
 			Server_helpers.exec_with_new_task "SR.create" ~subtask_of:(Ref.of_string dbg)
@@ -197,7 +190,7 @@ module SMAPIv1 = struct
 				virtual_size = vdi_rec.API.vDI_virtual_size;
 				physical_utilisation = vdi_rec.API.vDI_physical_utilisation;
 				persistent = vdi_rec.API.vDI_on_boot = `persist;
-				base_mirror = base_mirror vdi_rec;
+				sm_config = vdi_rec.API.vDI_sm_config;
 			}
 
 		let scan context ~dbg ~sr:sr' =
@@ -336,24 +329,29 @@ module SMAPIv1 = struct
                 read_only = r.API.vDI_read_only;
                 virtual_size = r.API.vDI_virtual_size;
                 physical_utilisation = r.API.vDI_physical_utilisation;
+<<<<<<< HEAD
 				persistent = r.API.vDI_on_boot = `persist;
 				base_mirror = base_mirror r;
+=======
+				sm_config = r.API.vDI_sm_config;
+>>>>>>> CA-78221: use sm-config instead of other-config
             }
 
         let newvdi ~__context vi =
             (* The current backends stash data directly in the db *)
             let uuid = require_uuid vi in
             vdi_info_from_db ~__context (Db.VDI.get_by_uuid ~__context ~uuid)
 
-        let create context ~dbg ~sr ~vdi_info ~params =
+        let create context ~dbg ~sr ~vdi_info =
             try
                 Server_helpers.exec_with_new_task "VDI.create" ~subtask_of:(Ref.of_string dbg)
                     (fun __context ->
+						let sm_config = vdi_info.sm_config in
                         let sr = Db.SR.get_by_uuid ~__context ~uuid:sr in
                         let vi =
                             Sm.call_sm_functions ~__context ~sR:sr
                                 (fun device_config _type ->
-                                    Sm.vdi_create device_config _type sr params vdi_info.ty
+                                    Sm.vdi_create device_config _type sr sm_config vdi_info.ty
                                         vdi_info.virtual_size vdi_info.name_label vdi_info.name_description
 										vdi_info.metadata_of_pool vdi_info.is_a_snapshot
 										vdi_info.snapshot_time vdi_info.snapshot_of vdi_info.read_only
@@ -364,13 +362,18 @@ module SMAPIv1 = struct
 				| Api_errors.Server_error(code, params) -> raise (Backend_error(code, params))
 				| Sm.MasterOnly -> redirect sr
 
-		let snapshot_and_clone call_name call_f context ~dbg ~sr ~vdi ~vdi_info ~params =
+		(* A list of keys in sm-config that will be preserved on clone/snapshot *)
+		let sm_config_keys_to_preserve_on_clone = [
+			"base_mirror"
+		]
+
+		let snapshot_and_clone call_name call_f context ~dbg ~sr ~vdi ~vdi_info =
 			try
 				Server_helpers.exec_with_new_task call_name ~subtask_of:(Ref.of_string dbg)
 					(fun __context ->
 						let vi = for_vdi ~dbg ~sr ~vdi call_name
 							(fun device_config _type sr self ->
-								call_f device_config _type params sr self
+								call_f device_config _type vdi_info.sm_config sr self
 							) in
 						(* PR-1255: modify clone, snapshot to take the same parameters as create? *)
 						let self, _ = find_vdi ~__context sr vi.Smint.vdi_info_location in
@@ -386,10 +389,12 @@ module SMAPIv1 = struct
 						Db.VDI.set_name_description ~__context ~self ~value:vdi_info.name_description;
 						Db.VDI.remove_from_other_config ~__context ~self ~key:"content_id";
 						Db.VDI.add_to_other_config ~__context ~self ~key:"content_id" ~value:content_id;
-						Opt.iter (fun id ->
-							Db.VDI.remove_from_other_config ~__context ~self ~key:"base_mirror";
-							Db.VDI.add_to_other_config ~__context ~self ~key:"base_mirror" ~value:id;
-						) vdi_info.base_mirror;
+						List.iter (fun (key, value) ->
+							if List.mem key sm_config_keys_to_preserve_on_clone then (
+								Db.VDI.remove_from_sm_config ~__context ~self ~key;
+								Db.VDI.add_to_sm_config ~__context ~self ~key ~value;
+							)
+						) vdi_info.sm_config;
 						for_vdi ~dbg ~sr ~vdi:vi.Smint.vdi_info_location "VDI.update"
 							(fun device_config _type sr self ->
 								Sm.vdi_update device_config _type sr self
@@ -553,9 +558,16 @@ module SMAPIv1 = struct
 					raise (Vdi_does_not_exist vdi1)
 				| Sm.MasterOnly -> redirect sr
 
-		let remove_from_other_config context ~dbg ~sr ~vdi ~key =
-			info "VDI.update_record dbg:%s sr:%s vdi:%s" dbg sr vdi;
-			Server_helpers.exec_with_new_task "VDI.update_record" ~subtask_of:(Ref.of_string dbg)
+		let add_to_sm_config context ~dbg ~sr ~vdi ~key ~value =
+			info "VDI.add_to_sm_config dbg:%s sr:%s vdi:%s key:%s value:%s" dbg sr vdi key value;
+			Server_helpers.exec_with_new_task "VDI.add_to_sm_config" ~subtask_of:(Ref.of_string dbg)
+				(fun __context ->
+					let self = find_vdi ~__context sr vdi |> fst in
+					Db.VDI.add_to_sm_config ~__context ~self ~key ~value)
+
+		let remove_from_sm_config context ~dbg ~sr ~vdi ~key =
+			info "VDI.remove_from_sm_config dbg:%s sr:%s vdi:%s key:%s" dbg sr vdi key;
+			Server_helpers.exec_with_new_task "VDI.remove_from_sm_config" ~subtask_of:(Ref.of_string dbg)
 				(fun __context ->
 					let self = find_vdi ~__context sr vdi |> fst in
 					Db.VDI.remove_from_other_config ~__context ~self ~key)

ocaml/xapi/storage_impl.ml

@@ -458,20 +458,20 @@ module Wrapper = functor(Impl: Server_impl) -> struct
 							Impl.VDI.epoch_end context ~dbg ~sr ~vdi
 						))
 
-        let create context ~dbg ~sr ~vdi_info ~params =
-            info "VDI.create dbg:%s sr:%s vdi_info:%s params:%s" dbg sr (string_of_vdi_info vdi_info) (String.concat "; " (List.map (fun (k, v) -> k ^ ":" ^ v) params));
-            let result = Impl.VDI.create context ~dbg ~sr ~vdi_info ~params in
+        let create context ~dbg ~sr ~vdi_info =
+            info "VDI.create dbg:%s sr:%s vdi_info:%s" dbg sr (string_of_vdi_info vdi_info);
+            let result = Impl.VDI.create context ~dbg ~sr ~vdi_info in
             match result with
                 | { virtual_size = virtual_size' } when virtual_size' < vdi_info.virtual_size ->
                     error "VDI.create dbg:%s created a smaller VDI (%Ld)" dbg virtual_size';
                     raise (Backend_error("SR_BACKEND_FAILURE", ["Disk too small"; Int64.to_string vdi_info.virtual_size; Int64.to_string virtual_size']))
                 | result -> result
 
-		let snapshot_and_clone call_name call_f context ~dbg ~sr ~vdi ~vdi_info ~params =
-			info "%s dbg:%s sr:%s vdi:%s vdi_info:%s params:%s" call_name dbg sr vdi (string_of_vdi_info vdi_info) (String.concat ";" (List.map (fun (k, v) -> k ^ ":" ^ v) params));
+		let snapshot_and_clone call_name call_f context ~dbg ~sr ~vdi ~vdi_info =
+			info "%s dbg:%s sr:%s vdi:%s vdi_info:%s" call_name dbg sr vdi (string_of_vdi_info vdi_info);
 			with_vdi sr vdi
 				(fun () ->
-					call_f context ~dbg ~sr ~vdi ~vdi_info ~params
+					call_f context ~dbg ~sr ~vdi ~vdi_info
 				)
 
 		let snapshot = snapshot_and_clone "VDI.snapshot" Impl.VDI.snapshot
@@ -515,9 +515,13 @@ module Wrapper = functor(Impl: Server_impl) -> struct
 			info "VDI.compose dbg:%s sr:%s vdi1:%s vdi2:%s" dbg sr vdi1 vdi2;
 			Impl.VDI.compose context ~dbg ~sr ~vdi1 ~vdi2
 
-		let remove_from_other_config context ~dbg ~sr ~vdi ~key =
+		let add_to_sm_config context ~dbg ~sr ~vdi ~key ~value =
+			info "VDI.add_to_other_config dbg:%s sr:%s vdi:%s key:%s valu:%s" dbg sr vdi key value;
+			Impl.VDI.add_to_sm_config context ~dbg ~sr ~vdi ~key ~value
+
+		let remove_from_sm_config context ~dbg ~sr ~vdi ~key =
 			info "VDI.remove_from_other_config dbg:%s sr:%s vdi:%s key:%s" dbg sr vdi key;
-			Impl.VDI.remove_from_other_config context ~dbg ~sr ~vdi ~key
+			Impl.VDI.remove_from_sm_config context ~dbg ~sr ~vdi ~key
 
 		let get_url context ~dbg ~sr ~vdi =
 			info "VDI.get_url dbg:%s sr:%s vdi:%s" dbg sr vdi;

ocaml/xapi/storage_impl_test.ml

@@ -54,10 +54,10 @@ module Debug_print_impl = struct
         let created = Hashtbl.create 10
 		let key_of sr vdi = Printf.sprintf "%s/%s" sr vdi
 
-        let create context ~dbg ~sr ~vdi_info ~params =
+        let create context ~dbg ~sr ~vdi_info =
             let vdi = "newvdi" in
             let info =
-                if List.mem_assoc "toosmall" params
+                if List.mem_assoc "toosmall" vdi_info.sm_config
                 then { vdi_info with virtual_size = Int64.sub vdi_info.virtual_size 1L }
                 else vdi_info in
             Mutex.execute m
@@ -67,8 +67,8 @@ module Debug_print_impl = struct
                 );
             info
 
-		let snapshot context ~dbg ~sr ~vdi ~vdi_info ~params =
-			create context ~dbg ~sr ~vdi_info ~params
+		let snapshot context ~dbg ~sr ~vdi ~vdi_info =
+			create context ~dbg ~sr ~vdi_info
 		let clone = snapshot
 
         let destroy context ~dbg ~sr ~vdi =
@@ -153,7 +153,8 @@ module Debug_print_impl = struct
 
         let get_url context ~dbg ~sr ~vdi = assert false
 		let compose context ~dbg ~sr ~vdi1 ~vdi2 = assert false
-		let remove_from_other_config context ~dbg ~sr ~vdi ~key = assert false
+		let add_to_sm_config context ~dbg ~sr ~vdi ~key ~value = assert false
+		let remove_from_sm_config context ~dbg ~sr ~vdi ~key = assert false
 		let set_content_id context ~dbg ~sr ~vdi ~content_id = assert false
 		let get_by_name context ~dbg ~sr ~name = assert false
 		let similar_content context ~dbg ~sr ~vdi = assert false
@@ -469,11 +470,13 @@ let create_vdi_test sr =
         physical_utilisation = 10L;
 		metadata_of_pool = "";
 		persistent = true;
-		base_mirror = None;
+		sm_config = [];
 	} in
     expect "too_small_backend_error" too_small_backend_error
-        (fun () -> Client.VDI.create ~dbg ~sr ~vdi_info ~params:["toosmall", ""]);
-    let vdi = Client.VDI.create ~dbg ~sr ~vdi_info ~params:[] in
+        (fun () ->
+			let vdi_info = { vdi_info with sm_config = ["toosmall", ""] } in
+			Client.VDI.create ~dbg ~sr ~vdi_info);
+    let vdi = Client.VDI.create ~dbg ~sr ~vdi_info in
     expect "attach_info" (fun _ -> true) 
 		(Client.VDI.attach ~dbg ~dp ~sr ~vdi:vdi.vdi ~read_write:false);
     debug "Detaching and cleaning up";

ocaml/xapi/storage_interface.ml

@@ -58,7 +58,7 @@ type vdi_info = {
     physical_utilisation: int64;
     (* xenstore_data: workaround via XenAPI *)
 	persistent: bool;
-    base_mirror: string option;
+    sm_config: (string * string) list;
 }
 
 let string_of_vdi_info (x: vdi_info) = Jsonrpc.to_string (rpc_of_vdi_info x)
@@ -237,16 +237,16 @@ module VDI = struct
 	(** Functions which operate on particular VDIs.
 		These functions are all idempotent from the point of view of a given [dp]. *)
 
-	(** [create task sr vdi_info params] creates a new VDI in [sr] using [vdi_info]. Some
+	(** [create task sr vdi_info] creates a new VDI in [sr] using [vdi_info]. Some
         fields in the [vdi_info] may be modified (e.g. rounded up), so the function
         returns the vdi_info which was used. *)
-	external create : dbg:debug_info -> sr:sr -> vdi_info:vdi_info -> params:(string*string) list -> vdi_info = ""
+	external create : dbg:debug_info -> sr:sr -> vdi_info:vdi_info -> vdi_info = ""
 
-	(** [snapshot task sr vdi vdi_info params] creates a new VDI which is a snapshot of [vdi] in [sr] *)
-	external snapshot : dbg:debug_info -> sr:sr -> vdi:vdi -> vdi_info:vdi_info -> params:(string*string) list -> vdi_info = ""
+	(** [snapshot task sr vdi vdi_info] creates a new VDI which is a snapshot of [vdi] in [sr] *)
+	external snapshot : dbg:debug_info -> sr:sr -> vdi:vdi -> vdi_info:vdi_info -> vdi_info = ""
 
-	(** [clone task sr vdi vdi_info params] creates a new VDI which is a clone of [vdi] in [sr] *)
-	external clone : dbg:debug_info -> sr:sr -> vdi:vdi -> vdi_info:vdi_info -> params:(string*string) list -> vdi_info = ""
+	(** [clone task sr vdi vdi_info] creates a new VDI which is a clone of [vdi] in [sr] *)
+	external clone : dbg:debug_info -> sr:sr -> vdi:vdi -> vdi_info:vdi_info -> vdi_info = ""
 
     (** [destroy task sr vdi] removes [vdi] from [sr] *)
     external destroy : dbg:debug_info -> sr:sr -> vdi:vdi -> unit = ""
@@ -297,8 +297,12 @@ module VDI = struct
     (** [compose task sr vdi1 vdi2] layers the updates from [vdi2] onto [vdi1], modifying [vdi2] *)
     external compose : dbg:debug_info -> sr:sr -> vdi1:vdi -> vdi2:vdi -> unit = ""
 
-	(** [remove_other_config dbg sr vdi key] remove [key] from [vdi] other config *)
-	external remove_from_other_config: dbg:debug_info -> sr:sr -> vdi:vdi -> key:string -> unit = ""
+	(** [add_to_sm_config dbg sr vdi key value] associates [value] to the [key] in [vdi] sm-config *)
+	external add_to_sm_config: dbg:debug_info -> sr:sr -> vdi:vdi -> key:string -> value:string -> unit = ""
+
+	(** [remove_from_sm_config dbg sr vdi key] remove [key] from [vdi] sm-config *)
+	external remove_from_sm_config: dbg:debug_info -> sr:sr -> vdi:vdi -> key:string -> unit = ""
+
 end
 
 (** [get_by_name task name] returns a vdi with [name] (which may be in any SR) *)