Check that there are no changes during SR.scan

Currently, we are only checking that no VDIs have been removed during the
SR scan performed by the SM plugin. However, there are situations where
a VDI has been added, and if this VDI is not present in the list obtained
from SR.scan, it will be forgotten. The checks only prevent this in the
case where the VDI was added during the scan. There is still a TOCTOU
situation if the issue happens after the scan, and there is room for that.

Signed-off-by: Guillaume <[email protected]>

Author: gthvn1

ocaml/xapi/xapi_sr.ml

@@ -29,6 +29,12 @@ open Xapi_database.Db_filter_types
 open API
 open Client
 
+module RefSet = Set.Make (struct
+  type t = [`VDI] Ref.t
+
+  let compare = Ref.compare
+end)
+
 (* internal api *)
 
 module D = Debug.Make (struct let name = "xapi_sr" end)
@@ -778,12 +784,19 @@ let scan ~__context ~sr =
               Db.VDI.get_records_where ~__context
                 ~expr:(Eq (Field "SR", Literal sr'))
             in
+            let string_of_l lst =
+              List.map (fun (_, v) -> v.vDI_uuid) lst |> String.concat ","
+            in
             (* It is sufficient to just compare the refs in two db_vdis, as this
                is what update_vdis uses to determine what to delete *)
             let vdis_ref_equal db_vdi1 db_vdi2 =
-              Listext.List.set_difference (List.map fst db_vdi1)
-                (List.map fst db_vdi2)
-              = []
+              let refs1 = RefSet.of_list (List.map fst db_vdi1) in
+              let refs2 = RefSet.of_list (List.map fst db_vdi2) in
+              (* VDIs that are in db_vdi1 but not in db_vdi2 *)
+              let vdis_removed = RefSet.diff refs1 refs2 in
+              (* VDIs that are in not in db_vdi1 but db_vdi2 *)
+              let vdis_added = RefSet.diff refs2 refs1 in
+              RefSet.is_empty vdis_removed && RefSet.is_empty vdis_added
             in
             let db_vdis_before = find_vdis () in
             let vs, sr_info =
@@ -797,12 +810,8 @@ let scan ~__context ~sr =
                 "%s detected db change while scanning, before scan vdis %s, \
                  after scan vdis %s, retry limit left %d"
                 __FUNCTION__
-                (List.map (fun (_, v) -> v.vDI_uuid) db_vdis_before
-                |> String.concat ","
-                )
-                (List.map (fun (_, v) -> v.vDI_uuid) db_vdis_after
-                |> String.concat ","
-                )
+                (string_of_l db_vdis_before)
+                (string_of_l db_vdis_after)
                 limit ;
               (scan_rec [@tailcall]) (limit - 1)
             ) else if limit = 0 then