oxenstored: handle unknown operations by returning an error to the client

Previous an unknown operation would be decoded as a Not_found exception which
would bubble all the way up to the try ... with surrounding the call to
main_loop where it would be logged and ignored.

This would leave the guest hanging waiting for a response to the invalid
request.

Instead introduce a specific "Invalid" operation. Higher level functionality,
such as Process.process_packet, already handles operations which are not
understood with an error reply due to the final wildcard entry in
Process.function_of_type but explicitly handle Invalid this way to make it
clear what is going on.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

5 files changed, 8 insertions, 7 deletions

diff --git a/tools/ocaml/libs/xb/op.ml b/tools/ocaml/libs/xb/op.ml
index 1ce72da348..0ee866676f 100644
--- a/tools/ocaml/libs/xb/op.ml
+++ b/tools/ocaml/libs/xb/op.ml
@@ -19,8 +19,7 @@ type operation = Debug | Directory | Read | Getperms |
                  Transaction_end | Introduce | Release |
                  Getdomainpath | Write | Mkdir | Rm |
                  Setperms | Watchevent | Error | Isintroduced |
-                 Resume | Set_target
-               | Restrict 
+                 Resume | Set_target | Restrict | Invalid
 
 let operation_c_mapping =
 	[| Debug; Directory; Read; Getperms;
@@ -41,7 +40,7 @@ let array_search el a =
 let of_cval i =
 	if i >= 0 && i < size
 	then operation_c_mapping.(i)
-	else raise Not_found
+	else Invalid
 
 let to_cval op =
 	array_search op operation_c_mapping
@@ -69,3 +68,4 @@ let to_string ty =
 	| Resume		-> "RESUME"
 	| Set_target		-> "SET_TARGET"
 	| Restrict		-> "RESTRICT"
+	| Invalid		-> "INVALID"
diff --git a/tools/ocaml/libs/xb/xb.mli b/tools/ocaml/libs/xb/xb.mli
index 1dde52db1e..58234aefed 100644
--- a/tools/ocaml/libs/xb/xb.mli
+++ b/tools/ocaml/libs/xb/xb.mli
@@ -23,6 +23,7 @@ module Op :
       | Resume
       | Set_target
       | Restrict
+      | Invalid (* Not a valid wire operation *)
     val operation_c_mapping : operation array
     val size : int
     val array_search : 'a -> 'a array -> int
diff --git a/tools/ocaml/xenstored/logging.ml b/tools/ocaml/xenstored/logging.ml
index 84d7c82cba..7152b4ec6c 100644
--- a/tools/ocaml/xenstored/logging.ml
+++ b/tools/ocaml/xenstored/logging.ml
@@ -182,6 +182,7 @@ let string_of_access_type = function
 
 	| Xenbus.Xb.Op.Error             -> "error    "
 	| Xenbus.Xb.Op.Watchevent        -> "w event  "
+	| Xenbus.Xb.Op.Invalid           -> "invalid  "
 	(*
 	| x                       -> Xenbus.Xb.Op.to_string x
 	*)
diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml
index c2aeaa94a4..a4ff741264 100644
--- a/tools/ocaml/xenstored/process.ml
+++ b/tools/ocaml/xenstored/process.ml
@@ -324,7 +324,8 @@ let function_of_type ty =
 	| Xenbus.Xb.Op.Resume            -> reply_ack do_resume
 	| Xenbus.Xb.Op.Set_target        -> reply_ack do_set_target
 	| Xenbus.Xb.Op.Restrict          -> reply_ack do_restrict
-	| _                       -> reply_ack do_error
+	| Xenbus.Xb.Op.Invalid           -> reply_ack do_error
+	| _                              -> reply_ack do_error
 
 let input_handle_error ~cons ~doms ~fct ~ty ~con ~t ~rid ~data =
 	let reply_error e =
diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml
index a08aa65d33..564dbeaddb 100644
--- a/tools/ocaml/xenstored/xenstored.ml
+++ b/tools/ocaml/xenstored/xenstored.ml
@@ -43,9 +43,7 @@ let process_connection_fds store cons domains rset wset =
 			debug "closing socket connection"
 		in
 	let process_fdset_with fds fct =
-		List.iter (fun fd ->
-		           try try_fct fct (Connections.find cons fd)
-		           with Not_found -> ()) fds
+		List.iter (fun fd -> try_fct fct (Connections.find cons fd)) fds
 	in
 	process_fdset_with rset Process.do_input;
 	process_fdset_with wset Process.do_output