Merge pull request #88 from xapi-project/more-tunnelling

Fix a race with lvchange -an

Author: djs55

test/test.ml

@@ -69,9 +69,40 @@ let lvcreate_l =
   assert_lv_exists ~expected_size_in_extents:1L "test";
   xenvm [ "lvremove"; vg ^ "/test" ] |> ignore_string
 
+let file_exists filename =
+  try
+    Unix.LargeFile.stat filename |> ignore;
+    true
+  with Unix.Unix_error(Unix.ENOENT, _, _) -> false
+
+let dm_exists name = match Devmapper.stat name with
+  | None -> false
+  | Some _ -> true
+
+let dev_path_of name = "/dev/" ^ vg ^ "/" ^ name
+let mapper_path_of name = "/dev/mapper/" ^ vg ^ "-" ^ name
+
+let lvchange_n =
+  "lvchange -an <device>: check that we can deactivate a volume" >::
+  fun () ->
+  xenvm [ "lvcreate"; "-n"; "test"; "-L"; "3"; vg ] |> ignore_string;
+  assert_lv_exists ~expected_size_in_extents:1L "test";
+  let vg_metadata, lv_metadata = Lwt_main.run (Client.get_lv "test") in
+  let name = Mapper.name_of vg_metadata lv_metadata in
+  xenvm [ "lvchange"; "-ay"; "/dev/" ^ vg ^ "/test" ] |> ignore_string;
+  assert_equal ~printer:string_of_bool true (file_exists (dev_path_of "test"));
+  assert_equal ~printer:string_of_bool true (file_exists (mapper_path_of "test"));
+  assert_equal ~printer:string_of_bool true (dm_exists name);
+  xenvm [ "lvchange"; "-an"; "/dev/" ^ vg ^ "/test" ] |> ignore_string;
+  assert_equal ~printer:string_of_bool false (file_exists (dev_path_of"test"));
+  assert_equal ~printer:string_of_bool false (file_exists (mapper_path_of"test"));
+  assert_equal ~printer:string_of_bool false (dm_exists name);
+  xenvm [ "lvremove"; vg ^ "/test" ] |> ignore_string
+
 let xenvmd_suite = "Commands which require xenvmd" >::: [
   lvcreate_L;
   lvcreate_l;
+  lvchange_n;
 ]
 
 let _ =

xenvm/lvchange.ml

@@ -43,9 +43,23 @@ let lvchange_activate copts vg_name lv_name physical_device =
 let deactivate vg lv =
   let open Xenvm_common in
   let name = Mapper.name_of vg lv in
-  let all = Devmapper.ls () in
-  if List.mem name all
-  then Devmapper.remove name;
+  (* This can fail with an EBUSY *)
+  let rec retry n =
+    let all = Devmapper.ls () in
+    let result =
+      try
+        if List.mem name all then Devmapper.remove name;
+        `Ok ()
+      with e ->
+        Printf.fprintf stderr "Caught %s while removing dm device %s\n%!" (Printexc.to_string e) name;
+        if n = 0 then raise e;
+        `Retry in
+    match result with
+    | `Ok () -> ()
+    | `Retry ->
+      Unix.sleep 1;
+      retry (n - 1) in
+  retry 30;
   (* Delete the device node *)
   let path = dev_path_of vg.Lvm.Vg.name lv.Lvm.Lv.name in
   Lwt.catch (fun () -> Lwt_unix.unlink path) (fun _ -> Lwt.return ()) >>= fun () ->