CA-209511: Don't remove private data path for devices during localhost migration

There can be multiple domains for a single VM during a localhost migration, and
the private data path is indexed by UUID, not domid. When unplugging the
devices for the old domain during a localhost migration, we must not remove the
private data keys, because they are needed by the new domain.

Signed-off-by: Rob Hoes <[email protected]>

Author: robhoes

xc/device.ml

@@ -449,12 +449,12 @@ let hard_shutdown_wait (task: Xenops_task.t) ~xs ~timeout x =
 	let (_: bool) = cancellable_watch (Device x) [ Watch.map (fun _ -> ()) (hard_shutdown_complete ~xs x) ] [] task ~xs ~timeout () in
 	()
 
-let release (task: Xenops_task.t) ~xs (x: device) =
+let release (task: Xenops_task.t) ~xc ~xs (x: device) =
 	debug "Device.Vbd.release %s" (string_of_device x);
 	(* Make sure blktap/blkback fire the udev remove event by deleting the
 	   backend now *)
 	Generic.safe_rm ~xs (backend_path_of_device ~xs x);
-	Hotplug.release task ~xs x;
+	Hotplug.release task ~xc ~xs x;
 
 	if !Xenopsd.run_hotplug_scripts
 	then Hotplug.run_hotplug_script x [ "remove" ];
@@ -555,7 +555,7 @@ let add_async ~xs ~hvm x domid =
 	Generic.add_device ~xs device back front priv [];
 	device
 
-let add_wait (task: Xenops_task.t) ~xs device =
+let add_wait (task: Xenops_task.t) ~xc ~xs device =
 	if !Xenopsd.run_hotplug_scripts
 	then Hotplug.run_hotplug_script device [ "add" ];
 
@@ -577,15 +577,15 @@ let add_wait (task: Xenops_task.t) ~xs device =
 	  with Hotplug.Frontend_device_error _ as e ->
 	    debug "Caught Frontend_device_error: assuming it is safe to shutdown the backend";
 	    clean_shutdown task ~xs device; (* assumes double-failure isn't possible *)
-	    release task ~xs device;
+	    release task ~xc ~xs device;
 	    raise e
 	end;
 	device
 
 (* Add the VBD to the domain, When this command returns, the device is ready. (This isn't as
    concurrent as xend-- xend allocates loopdevices via hotplug in parallel and then
    performs a 'waitForDevices') *)
-let add (task: Xenops_task.t) ~xs ~hvm x domid =
+let add (task: Xenops_task.t) ~xc ~xs ~hvm x domid =
 	let device =
 		let result = ref None in
 		while !result = None do
@@ -597,7 +597,7 @@ let add (task: Xenops_task.t) ~xs ~hvm x domid =
 					Thread.delay 0.1
 				end else raise e (* permanent failure *)
 		done; Opt.unbox !result in
-	add_wait task ~xs device
+	add_wait task ~xc ~xs device
 
 let qemu_media_change ~xs device _type params =
 	let backend_path  = (backend_path_of_device ~xs device) in
@@ -739,15 +739,15 @@ let set_carrier ~xs (x: device) carrier =
 	let disconnect_path = disconnect_path_of_device ~xs x in
 	xs.Xs.write disconnect_path (if carrier then "0" else "1")
 
-let release (task: Xenops_task.t) ~xs (x: device) =
+let release (task: Xenops_task.t) ~xc ~xs (x: device) =
 	debug "Device.Vif.release %s" (string_of_device x);
 
 	if !Xenopsd.run_hotplug_scripts then begin
 		let tap = { x with backend = { x.backend with kind = Tap } } in
 		Hotplug.run_hotplug_script x [ "remove" ];
 		Hotplug.run_hotplug_script tap [ "remove" ];
 	end;
-	Hotplug.release task ~xs x
+	Hotplug.release task ~xc ~xs x
 
 let move ~xs (x: device) bridge =
 	let xs_bridge_path = Device_common.get_private_data_path_of_device x ^ "/bridge" in

xc/device.mli

@@ -57,9 +57,9 @@ sig
 		backend_domid: int;
 	}
 
-	val add : Xenops_task.t -> xs:Xenstore.Xs.xsh -> hvm:bool -> t -> Xenctrl.domid -> device
+	val add : Xenops_task.t -> xc:Xenctrl.handle -> xs:Xenstore.Xs.xsh -> hvm:bool -> t -> Xenctrl.domid -> device
 
-	val release : Xenops_task.t -> xs:Xenstore.Xs.xsh -> device -> unit
+	val release : Xenops_task.t -> xc:Xenctrl.handle -> xs:Xenstore.Xs.xsh -> device -> unit
 	val media_eject : xs:Xenstore.Xs.xsh -> device -> unit
 	val media_insert : xs:Xenstore.Xs.xsh -> phystype:physty -> params:string -> device -> unit
 	val media_is_ejected : xs:Xenstore.Xs.xsh -> device -> bool
@@ -84,7 +84,7 @@ sig
 	       -> ?extra_xenserver_keys:(string * string) list -> Xenctrl.domid
 	       -> device
 	val set_carrier : xs:Xenstore.Xs.xsh -> device -> bool -> unit
-	val release : Xenops_task.t -> xs:Xenstore.Xs.xsh -> device -> unit
+	val release : Xenops_task.t -> xc:Xenctrl.handle -> xs:Xenstore.Xs.xsh -> device -> unit
 	val move : xs:Xenstore.Xs.xsh -> device -> string -> unit
 end
 

xc/device_common.ml

@@ -132,12 +132,10 @@ let string_of_device (x: device) =
  * to take the UUID as an argument (and change the callers as well...) *)
 let uuid_of_domid domid =
 	try
-		Xenops_helpers.with_xc_and_xs (fun _ xs ->
-			let vm = xs.Xs.getdomainpath domid ^ "/vm" in
-			let vm_dir = xs.Xs.read vm in
-			xs.Xs.read (vm_dir ^ "/uuid")
+		with_xs (fun xs ->
+			Uuidm.to_string (Xenops_helpers.uuid_of_domid ~xs domid)
 		)
-	with _ ->
+	with Xenops_helpers.Domain_not_found ->
 		error "uuid_of_domid failed for domid %d" domid;
 		(* Returning a random string on error is not very neat, but we must avoid
 		 * exceptions here. This patch must be followed soon by a patch that changes the

xc/domain.ml

@@ -368,8 +368,7 @@ let destroy (task: Xenops_task.t) ~xc ~xs ~qemu_domid domid =
 
 	(* Any other domains with the same UUID as the one we are destroying.
 	 * There can be one during a localhost migration. *)
-	let other_domains = List.filter (fun x -> Xenctrl_uuid.uuid_of_handle x.Xenctrl.handle = uuid)
-		(Xenctrl.domain_getinfolist xc 0) in
+	let other_domains = Xenops_helpers.domains_of_uuid ~xc uuid in
 	debug "VM = %s; domid = %d; Domain.destroy: other domains with the same UUID = [ %a ]"
 		(Uuid.to_string uuid) domid
 		(fun () -> String.concat "; ")
@@ -423,7 +422,7 @@ let destroy (task: Xenops_task.t) ~xc ~xs ~qemu_domid domid =
 	List.iter (fun x ->
 		log_exn_continue ("waiting for hotplug for " ^ (string_of_device x))
 		                 (fun () ->
-			Hotplug.release task ~xs x; released := x :: !released
+			Hotplug.release task ~xc ~xs x; released := x :: !released
 		                 ) ()
 		) all_devices;
 

xc/hotplug.ml

@@ -196,15 +196,24 @@ let wait_for_connect (task: Xenops_task.t) ~xs (x: device) =
 (* Wait for the device to be released by the backend driver (via udev) and
    then deallocate any resources which are registered (in our private bit of
    xenstore) *)
-let release (task:Xenops_task.t) ~xs (x: device) =
+let release (task:Xenops_task.t) ~xc ~xs (x: device) =
 	debug "Hotplug.release: %s" (string_of_device x);
 	wait_for_unplug task ~xs x;
 	let hotplug_path = get_hotplug_path x in
-	let private_data_path = get_private_data_path_of_device x in
+	let private_data_path =
+		(* Only remove the private data path if there isn't another domain on the host for the same VM.
+		 * There can be multiple during a localhost migration, and the private path is indexed by UUID, not domid. *)
+		let vm_uuid = Xenops_helpers.uuid_of_domid ~xs x.frontend.domid in
+		let domains_of_vm = Xenops_helpers.domains_of_uuid ~xc vm_uuid in
+		if List.length domains_of_vm <= 1 then
+			Some (get_private_data_path_of_device x)
+		else
+			None
+	in
 	let extra_xenserver_path = extra_xenserver_path_of_device xs x in
 	Xs.transaction xs (fun t ->
 		t.Xst.rm hotplug_path;
-		t.Xst.rm private_data_path;
+		Opt.iter t.Xst.rm private_data_path;
 		t.Xst.rm extra_xenserver_path
 	)
 

xc/xenops_helpers.ml

@@ -24,3 +24,20 @@ let with_xc_and_xs f =
 let with_xc_and_xs_final f cf =
 	with_xc_and_xs (fun xc xs -> finally (fun () -> f xc xs) cf)
 
+exception Domain_not_found
+
+let uuid_of_domid ~xs domid =
+	try
+		let vm = xs.Xs.getdomainpath domid ^ "/vm" in
+		let vm_dir = xs.Xs.read vm in
+		match Uuidm.of_string (xs.Xs.read (vm_dir ^ "/uuid")) with
+		| Some uuid -> uuid
+		| None -> raise Domain_not_found
+	with _ ->
+		raise Domain_not_found
+
+let domains_of_uuid ~xc uuid =
+	List.filter
+		(fun x -> Xenctrl_uuid.uuid_of_handle x.Xenctrl.handle = uuid)
+		(Xenctrl.domain_getinfolist xc 0)
+

xc/xenops_server_xen.ml

@@ -228,7 +228,7 @@ let create_vbd_frontend ~xc ~xs task frontend_domid vdi =
 				backend_domid = backend_domid;
 			} in
 			let device = Xenops_task.with_subtask task "Vbd.add"
-				(fun () -> Device.Vbd.add task ~xs ~hvm:false t frontend_domid) in
+				(fun () -> Device.Vbd.add task ~xc ~xs ~hvm:false t frontend_domid) in
 			Device device
 
 let block_device_of_vbd_frontend = function
@@ -1987,7 +1987,7 @@ module VBD = struct
 					} in
 					let device =
 						Xenops_task.with_subtask task (Printf.sprintf "Vbd.add %s" (id_of vbd))
-							(fun () -> Device.Vbd.add task ~xs ~hvm x frontend_domid) in
+							(fun () -> Device.Vbd.add task ~xc ~xs ~hvm x frontend_domid) in
 
 					(* We store away the disk so we can implement VBD.stat *)
 					Opt.iter (fun disk -> xs.Xs.write (vdi_path_of_device ~xs device) (disk |> rpc_of_disk |> Jsonrpc.to_string)) vbd.backend;
@@ -2063,7 +2063,7 @@ module VBD = struct
 							Opt.iter
 								(fun device ->
 									Xenops_task.with_subtask task (Printf.sprintf "Vbd.release %s" (id_of vbd))
-										(fun () -> Device.Vbd.release task ~xs device);
+										(fun () -> Device.Vbd.release task ~xc ~xs device);
 								) device;
 							(* If we have a qemu frontend, detach this too. *)
 							Mutex.execute dB_m (fun () ->
@@ -2356,7 +2356,7 @@ module VIF = struct
 						Xenops_task.with_subtask task (Printf.sprintf "Vif.hard_shutdown %s" (id_of vif))
 							(fun () -> (if force then Device.hard_shutdown else Device.clean_shutdown) task ~xs device);
 						Xenops_task.with_subtask task (Printf.sprintf "Vif.release %s" (id_of vif))
-							(fun () -> Device.Vif.release task ~xs device) in
+							(fun () -> Device.Vif.release task ~xc ~xs device) in
 					destroy device;
 
 					Opt.iter (fun vm_t ->