tools/oxenstored: Use more efficient node trees

This changes the output of xenstore-ls to be sorted.  Previously the keys were
listed in the order in which they were inserted in.  docs/misc/xenstore.txt
doesn't specify in what order keys are listed.

Map.update is used to retain semantics with replace_child: only an existing
child is replaced, if it wasn't part of the original map we don't add it.
Similarly exception behaviour is retained for del_childname and related
functions.

Entries are stored in reverse sort order, so that upon Map.fold the
constructed list is sorted in ascending order and there is no need for a
List.rev.

This changes the semantics and is not suitable as is for a backport.  It
reveals bugs in buggy clients that depend on xenstore entry order, however
those clients should be fixed.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>

diff --git a/tools/ocaml/xenstored/store.ml b/tools/ocaml/xenstored/store.ml
index 9c226e4ef7ff256efd70da88ed9921721199905a..1bd0c81f6f6afd336c8a78f5629a8dd05f9c9197 100644 (file)
--- a/tools/ocaml/xenstored/store.ml
+++ b/tools/ocaml/xenstored/store.ml
@@ -16,17 +16,19 @@
  *)
 open Stdext
 
+module SymbolMap = Map.Make(Symbol)
+
 module Node = struct
 
 type t = {
        name: Symbol.t;
        perms: Perms.Node.t;
        value: string;
-       children: t list;
+       children: t SymbolMap.t;
 }
 
 let create _name _perms _value =
-       { name = Symbol.of_string _name; perms = _perms; value = _value; children = []; }
+       { name = Symbol.of_string _name; perms = _perms; value = _value; children = SymbolMap.empty; }
 
 let get_owner node = Perms.Node.get_owner node.perms
 let get_children node = node.children
@@ -42,38 +44,34 @@ let set_value node nvalue =
 let set_perms node nperms = { node with perms = nperms }
 
 let add_child node child =
-       { node with children = child :: node.children }
+       let children = SymbolMap.add child.name child node.children in
+       { node with children }
 
 let exists node childname =
        let childname = Symbol.of_string childname in
-       List.exists (fun n -> Symbol.equal n.name childname) node.children
+       SymbolMap.mem childname node.children
 
 let find node childname =
        let childname = Symbol.of_string childname in
-       List.find (fun n -> Symbol.equal n.name childname) node.children
+       SymbolMap.find childname node.children
 
 let replace_child node child nchild =
-       (* this is the on-steroid version of the filter one-replace one *)
-       let rec replace_one_in_list l =
-               match l with
-               | []                               -> []
-               | h :: tl when Symbol.equal h.name child.name -> nchild :: tl
-               | h :: tl                          -> h :: replace_one_in_list tl
-               in
-       { node with children = (replace_one_in_list node.children) }
+       { node with
+         children = SymbolMap.update child.name
+                       (function None -> None | Some _ -> Some nchild)
+                       node.children
+       }
 
 let del_childname node childname =
        let sym = Symbol.of_string childname in
-       let rec delete_one_in_list l =
-               match l with
-               | []                        -> raise Not_found
-               | h :: tl when Symbol.equal h.name sym -> tl
-               | h :: tl                   -> h :: delete_one_in_list tl
-               in
-       { node with children = (delete_one_in_list node.children) }
+       { node with children =
+               SymbolMap.update sym
+                       (function None -> raise Not_found | Some _ -> None)
+                       node.children
+       }
 
 let del_all_children node =
-       { node with children = [] }
+       { node with children = SymbolMap.empty }
 
 (* check if the current node can be accessed by the current connection with rperm permissions *)
 let check_perm node connection request =
@@ -87,12 +85,12 @@ let check_owner node connection =
                raise Define.Permission_denied;
        end
 
-let rec recurse fct node = fct node; List.iter (recurse fct) node.children
+let rec recurse fct node = fct node; SymbolMap.iter (fun _ -> recurse fct) node.children
 
 (** [recurse_map f tree] applies [f] on each node in the tree recursively *)
 let recurse_map f =
        let rec walk node =
-               f { node with children = List.rev_map walk node.children |> List.rev }
+               f { node with children = SymbolMap.map walk node.children }
        in
        walk
 
@@ -336,7 +334,7 @@ let ls store perm path =
                                Node.check_perm cnode perm Perms.READ;
                                cnode.Node.children in
                        Path.apply store.root path do_ls in
-       List.rev (List.map (fun n -> Symbol.to_string n.Node.name) children)
+       SymbolMap.fold (fun k _ accu -> Symbol.to_string k :: accu) children []
 
 let getperms store perm path =
        if path = [] then (
@@ -366,7 +364,7 @@ let traversal root_node f =
        let rec _traversal path node =
                f path node;
                let node_path = Path.of_path_and_name path (Symbol.to_string node.Node.name) in
-               List.iter (_traversal node_path) (List.rev node.Node.children)
+               SymbolMap.iter (fun _ -> _traversal node_path) node.Node.children
                in
        _traversal [] root_node
 

diff --git a/tools/ocaml/xenstored/symbol.ml b/tools/ocaml/xenstored/symbol.ml
index 2b41d120f66ebb9a61b44787d4ea7866e49e3e64..301639f16f41a6983177c88d8cad11978a6ef38e 100644 (file)
--- a/tools/ocaml/xenstored/symbol.ml
+++ b/tools/ocaml/xenstored/symbol.ml
@@ -31,6 +31,10 @@ let equal a b =
   (* compare using physical equality, both members have to be part of the above weak table *)
   a == b
 
+let compare a b =
+  if equal a b then 0
+  else -(String.compare a b)
+
 let stats () =
   let len, entries, _, _, _, _ = WeakTable.stats tbl in
   len, entries

diff --git a/tools/ocaml/xenstored/symbol.mli b/tools/ocaml/xenstored/symbol.mli
index 586ab5750767a91b57283243a18832fe2d44e0a0..dd0f01479655a1ebba8540da387112096193618a 100644 (file)
--- a/tools/ocaml/xenstored/symbol.mli
+++ b/tools/ocaml/xenstored/symbol.mli
@@ -32,6 +32,9 @@ val to_string : t -> string
 val equal: t -> t -> bool
 (** Compare two symbols for equality *)
 
+val compare: t -> t -> int
+(** Compare two symbols *)
+
 (** {6 Statistics } *)
 
 val stats : unit -> int * int