Do not log "get" commands

To reduce the amount of spam in the logs, only log the commands that change
something in the system (e.g. `ip addr add`) and not the ones that query
something (e.g. `ip addr show`). We still log it if the command fails.

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

Author: robhoes

lib/network_utils.ml

@@ -75,12 +75,13 @@ let default_error_handler script args stdout stderr status =
                                       "stdout", stdout;
                                       "stderr", stderr]))
 
-let check_n_run ?(on_error=default_error_handler) run_func script args =
+let check_n_run ?(on_error=default_error_handler) ?(log=true) run_func script args =
   try
     Unix.access script [ Unix.X_OK ];
     (* Use the same $PATH as xapi *)
     let env = [| "PATH=" ^ (Sys.getenv "PATH") |] in
-    info "%s %s" script (String.concat " " args);
+    if log then
+      info "%s %s" script (String.concat " " args);
     run_func env script args
   with
   | Unix.Unix_error (e, a, b) ->
@@ -90,19 +91,19 @@ let check_n_run ?(on_error=default_error_handler) run_func script args =
   | Forkhelpers.Spawn_internal_error(stderr, stdout, status)->
     on_error script args stdout stderr status
 
-let call_script ?(timeout=Some 60.0) ?on_error script args =
+let call_script ?(timeout=Some 60.0) ?on_error ?log script args =
   let call_script_internal env script args =
     let (out,err) = Forkhelpers.execute_command_get_output ~env ?timeout script args in
     out
   in
-  check_n_run ?on_error call_script_internal script args
+  check_n_run ?on_error ?log call_script_internal script args
 
-let fork_script ?on_error script args =
+let fork_script ?on_error ?log script args =
   let fork_script_internal env script args =
     let pid = Forkhelpers.safe_close_and_exec ~env None None None [] script args in
     Forkhelpers.dontwaitpid pid;
   in
-  check_n_run ?on_error fork_script_internal script args
+  check_n_run ?on_error ?log fork_script_internal script args
 
 module Sysfs = struct
   let list () =
@@ -357,19 +358,17 @@ module Ip = struct
     | V6 -> ["-6"]
     | V46 -> []
 
-  let call args =
-    call_script iproute2 args
+  let call ?log args =
+    call_script ?log iproute2 args
 
   let find output attr =
-    info "Looking for %s in [%s]" attr output;
     let args = Astring.String.fields ~empty:false output in
     let indices = (Xapi_stdext_std.Listext.List.position (fun s -> s = attr) args) in
-    info "Found at [ %s ]" (String.concat ", " (List.map string_of_int indices));
     List.map (fun i -> List.nth args (succ i)) indices
 
   let get_link_flags dev =
     Sysfs.assert_exists dev;
-    let output = call ["link"; "show"; "dev"; dev] in
+    let output = call ~log:false ["link"; "show"; "dev"; dev] in
     let i = String.index output '<' in
     let j = String.index output '>' in
     let flags = String.sub output (i + 1) (j - i - 1) in
@@ -406,13 +405,13 @@ module Ip = struct
   let link ?(version=V46) dev attr =
     Sysfs.assert_exists dev;
     let v = string_of_version version in
-    let output = call (v @ ["link"; "show"; "dev"; dev]) in
+    let output = call ~log:false (v @ ["link"; "show"; "dev"; dev]) in
     find output attr
 
   let addr ?(version=V46) dev attr =
     Sysfs.assert_exists dev;
     let v = string_of_version version in
-    let output = call (v @ ["addr"; "show"; "dev"; dev]) in
+    let output = call ~log:false (v @ ["addr"; "show"; "dev"; dev]) in
     find output attr
 
   let get_mtu dev =
@@ -517,7 +516,7 @@ module Ip = struct
 
   let route_show ?(version=V46) dev =
     let v = string_of_version version in
-    call (v @ ["route"; "show"; "dev"; dev])
+    call ~log:false (v @ ["route"; "show"; "dev"; dev])
 
   let set_route ?network dev gateway =
     try
@@ -819,12 +818,12 @@ module Dhclient = struct
 end
 
 module Fcoe = struct
-  let call args =
-    call_script ~timeout:(Some 10.0) !fcoedriver args
+  let call ?log args =
+    call_script ?log ~timeout:(Some 10.0) !fcoedriver args
 
   let get_capabilities name =
     try
-      let output = call ["--xapi"; name; "capable"] in
+      let output = call ~log:false ["--xapi"; name; "capable"] in
       if Astring.String.is_infix ~affix:"True" output then ["fcoe"] else []
     with _ ->
       debug "Failed to get fcoe support status on device %s" name;
@@ -929,20 +928,20 @@ module Ovs = struct
       default_error_handler script args stdout stderr exn
 
   module Cli : sig
-    val vsctl : string list -> string
-    val ofctl : string list -> string
-    val appctl : string list -> string
+    val vsctl : ?log:bool -> string list -> string
+    val ofctl : ?log:bool -> string list -> string
+    val appctl : ?log:bool -> string list -> string
   end = struct
     open Xapi_stdext_threads
     let s = Semaphore.create 5
-    let vsctl args =
+    let vsctl ?log args =
       Semaphore.execute s (fun () ->
-          call_script ~on_error:error_handler ovs_vsctl ("--timeout=20" :: args)
+          call_script ~on_error:error_handler ?log ovs_vsctl ("--timeout=20" :: args)
         )
-    let ofctl args =
-      call_script ~on_error:error_handler ovs_ofctl args
-    let appctl args =
-      call_script ~on_error:error_handler ovs_appctl args
+    let ofctl ?log args =
+      call_script ~on_error:error_handler ?log ovs_ofctl args
+    let appctl ?log args =
+      call_script ~on_error:error_handler ?log ovs_appctl args
   end
 
   module type Cli_S = module type of Cli
@@ -958,15 +957,15 @@ module Ovs = struct
           let raw_list = (Astring.String.cuts ~empty:false ~sep:"," (String.sub raw 1 (String.length raw - 2))) in
           let uuids = List.map (String.trim) raw_list in
           List.map (fun uuid ->
-              let raw = String.trim (vsctl ["get"; "interface"; uuid; "name"]) in
+              let raw = String.trim (vsctl ~log:false ["get"; "interface"; uuid; "name"]) in
               String.sub raw 1 (String.length raw - 2)) uuids
         else
           []
       with _ -> []
 
     let bridge_to_ports name =
       try
-        let ports = String.trim (vsctl ["list-ports"; name]) in
+        let ports = String.trim (vsctl ~log:false ["list-ports"; name]) in
         let ports' =
           if ports <> "" then
             Astring.String.cuts ~empty:false ~sep:"\n" ports
@@ -978,7 +977,7 @@ module Ovs = struct
 
     let bridge_to_interfaces name =
       try
-        let ifaces = String.trim (vsctl ["list-ifaces"; name]) in
+        let ifaces = String.trim (vsctl ~log:false ["list-ifaces"; name]) in
         if ifaces <> "" then
           Astring.String.cuts ~empty:false ~sep:"\n" ifaces
         else
@@ -987,8 +986,8 @@ module Ovs = struct
 
     let bridge_to_vlan name =
       try
-        let parent = vsctl ["br-to-parent"; name] |> String.trim in
-        let vlan = vsctl ["br-to-vlan"; name] |> String.trim |> int_of_string in
+        let parent = vsctl ~log:false ["br-to-parent"; name] |> String.trim in
+        let vlan = vsctl ~log:false ["br-to-vlan"; name] |> String.trim |> int_of_string in
         Some (parent, vlan)
       with e ->
         debug "bridge_to_vlan: %s" (Printexc.to_string e);
@@ -1001,7 +1000,7 @@ module Ovs = struct
 
     let get_bond_link_status name =
       try
-        let raw = appctl ["bond/show"; name] in
+        let raw = appctl ~log:false ["bond/show"; name] in
         let lines = Astring.String.cuts ~empty:false ~sep:"\n" raw in
         List.fold_left (fun (slaves, active_slave) line ->
             let slaves =
@@ -1027,7 +1026,7 @@ module Ovs = struct
 
     let get_bond_mode name =
       try
-        let output = String.trim (vsctl ["get"; "port"; name; "bond_mode"]) in
+        let output = String.trim (vsctl ~log:false ["get"; "port"; name; "bond_mode"]) in
         if output <> "[]" then Some output else None
       with _ ->
         None
@@ -1076,15 +1075,15 @@ module Ovs = struct
     let get_vlans name =
       try
         let vlans_with_uuid =
-          let raw = vsctl ["--bare"; "-f"; "table"; "--"; "--columns=name,_uuid"; "find"; "port"; "fake_bridge=true"] in
+          let raw = vsctl ~log:false ["--bare"; "-f"; "table"; "--"; "--columns=name,_uuid"; "find"; "port"; "fake_bridge=true"] in
           if raw <> "" then
             let lines = Astring.String.cuts ~empty:false ~sep:"\n" (String.trim raw) in
             List.map (fun line -> Scanf.sscanf line "%s %s" (fun a b-> a, b)) lines
           else
             []
         in
         let bridge_ports =
-          let raw = vsctl ["get"; "bridge"; name; "ports"] in
+          let raw = vsctl ~log:false ["get"; "bridge"; name; "ports"] in
           let raw = String.trim raw in
           if raw <> "[]" then
             let raw_list = (Astring.String.cuts ~empty:false ~sep:"," (String.sub raw 1 (String.length raw - 2))) in
@@ -1106,7 +1105,7 @@ module Ovs = struct
 
     let get_mcast_snooping_enable ~name =
       try
-        vsctl ["--"; "get"; "bridge"; name; "mcast_snooping_enable"]
+        vsctl ~log:false ["--"; "get"; "bridge"; name; "mcast_snooping_enable"]
         |> String.trim
         |> bool_of_string
       with _ -> false
@@ -1190,7 +1189,7 @@ module Ovs = struct
       vsctl ["--"; "--if-exists"; "del-br"; name]
 
     let list_bridges () =
-      let bridges = String.trim (vsctl ["list-br"]) in
+      let bridges = String.trim (vsctl ~log:false ["list-br"]) in
       if bridges <> "" then
         Astring.String.cuts ~empty:false ~sep:"\n" bridges
       else
@@ -1205,7 +1204,7 @@ module Ovs = struct
       vsctl ["--"; "--with-iface"; "--if-exists"; "del-port"; name]
 
     let port_to_bridge name =
-      vsctl ["port-to-br"; name]
+      vsctl ~log:false ["port-to-br"; name]
 
     let make_bond_properties name properties =
       let known_props = ["mode"; "hashing-algorithm"; "updelay"; "downdelay";
@@ -1285,7 +1284,7 @@ module Ovs = struct
                        mac_args @ args @ per_iface_args)
 
     let get_fail_mode bridge =
-      vsctl ["get-fail-mode"; bridge]
+      vsctl ~log:false ["get-fail-mode"; bridge]
 
     let add_default_flows bridge mac interfaces =
       let ports = List.map (fun interface -> vsctl ["get"; "interface"; interface; "ofport"]) interfaces in

test/network_test_lacp_properties.ml

@@ -70,7 +70,7 @@ let test_lacp_aggregation_key arg () =
 module OVS_Cli_test = struct
   include Ovs.Cli
   let vsctl_output = ref []
-  let vsctl args =
+  let vsctl ?log args =
     vsctl_output := args ;
     String.concat " " args
 end