Fixup: restruct some functions in networkd

- combine sorting on first boot and non first boot in one function
- reduce the use of device_already_renamed and make it clear
  that interface_order is None when device_already_renamed

Signed-off-by: Changlei Li <[email protected]>

Author: changlei-li

ocaml/networkd/bin/network_server.ml

@@ -35,74 +35,62 @@ let write_config () =
     try Network_config.write_config !config
     with Network_config.Write_error -> ()
 
-let sort_and_update () =
-  if Network_utils.device_already_renamed then
-    ()
-  else
-    let last_config = !config in
-    let last_interface_order =
-      Option.value ~default:[] last_config.interface_order
-    in
-    match Network_device_order.sort last_interface_order with
-    | Ok (new_order, changed_interfaces) ->
-        let update_name name =
-          List.assoc_opt name changed_interfaces |> Option.value ~default:name
-        in
-        let update_port (port, port_conf) =
-          ( update_name port
-          , {
-              port_conf with
-              interfaces= List.map update_name port_conf.interfaces
-            }
-          )
-        in
-        let bridge_config =
-          List.map
-            (fun (bridge, bridge_conf) ->
-              ( bridge
-              , {bridge_conf with ports= List.map update_port bridge_conf.ports}
-              )
-            )
-            last_config.bridge_config
-        in
-        let interface_config =
-          List.map
-            (fun (name, conf) -> (update_name name, conf))
-            last_config.interface_config
-        in
-        config :=
-          {
-            last_config with
-            interface_config
-          ; bridge_config
-          ; interface_order= Some new_order
-          }
-    | Error err ->
-        error "Failed to sort interface order [%s]"
-          (Network_device_order.string_of_error err)
-
-let sort_on_first_boot () =
-  if Network_utils.device_already_renamed then
-    None
-  else
-    match Network_device_order.sort [] with
-    | Ok (sorted, _) ->
-        Some sorted
+let sort last_order =
+  match last_order with
+  | Some last_order -> (
+    match Network_device_order.sort last_order with
+    | Ok (interface_order, changes) ->
+        (Some interface_order, changes)
     | Error err ->
         error "Failed to sort interface order [%s]"
           (Network_device_order.string_of_error err) ;
-        Some []
+        (Some last_order, [])
+  )
+  | None ->
+      (None, [])
 
-let build_config () =
+let update_changes last_config changed_interfaces =
+  let update_name name =
+    let new_name =
+      List.assoc_opt name changed_interfaces |> Option.value ~default:name
+    in
+    if name <> new_name then
+      debug "Renaming %s to %s" name new_name ;
+    new_name
+  in
+  let update_port (port, port_conf) =
+    ( update_name port
+    , {port_conf with interfaces= List.map update_name port_conf.interfaces}
+    )
+  in
+  let bridge_config =
+    List.map
+      (fun (bridge, bridge_conf) ->
+        ( bridge
+        , {bridge_conf with ports= List.map update_port bridge_conf.ports}
+        )
+      )
+      last_config.bridge_config
+  in
+  let interface_config =
+    List.map
+      (fun (name, conf) -> (update_name name, conf))
+      last_config.interface_config
+  in
+  (bridge_config, interface_config)
+
+let read_config () =
   try
     config := Network_config.read_config () ;
     debug "Read configuration from networkd.db file." ;
-    sort_and_update ()
+    let interface_order, changes = sort !config.interface_order in
+    let bridge_config, interface_config = update_changes !config changes in
+    config := {!config with bridge_config; interface_config; interface_order}
   with Network_config.Read_error -> (
     try
       (* No configuration file found. Try to get the initial network setup from
        * the first-boot data written by the host installer. *)
-      let interface_order = sort_on_first_boot () in
+      let interface_order, _ = sort Network_config.initial_interface_order in
       config := Network_config.read_management_conf interface_order ;
       debug "Read configuration from management.conf file."
     with Network_config.Read_error ->
@@ -148,7 +136,7 @@ let sync_state () =
   write_config ()
 
 let reset_state () =
-  let interface_order = sort_on_first_boot () in
+  let interface_order, _ = sort Network_config.initial_interface_order in
   config := Network_config.read_management_conf interface_order
 
 let set_gateway_interface _dbg name =
@@ -352,16 +340,18 @@ module Interface = struct
   let get_interface_positions dbg () =
     Debug.with_thread_associated dbg
       (fun () ->
-        if Network_utils.device_already_renamed then
-          sort_based_on_ethx ()
-        else
-          Option.value ~default:[] !config.interface_order
-          |> List.filter_map (fun dev ->
-                 if dev.present then
-                   Some (dev.name, dev.position)
-                 else
-                   None
-             )
+        match !config.interface_order with
+        | Some order ->
+            List.filter_map
+              (fun dev ->
+                if dev.present then
+                  Some (dev.name, dev.position)
+                else
+                  None
+              )
+              order
+        | None ->
+            sort_based_on_ethx ()
       )
       ()
 
@@ -1667,7 +1657,7 @@ let on_startup () =
       in
       try
         (* the following is best-effort *)
-        build_config () ;
+        read_config () ;
         remove_centos_config () ;
         if !backend_kind = Openvswitch then
           Ovs.set_max_idle 5000 ;

ocaml/networkd/lib/network_config.ml

@@ -22,7 +22,18 @@ exception Read_error
 
 exception Write_error
 
-let empty_config = default_config
+(* If the interface-rename script dir exists, the devices are already renamed
+   to eth<N>, the <N> indicates device order *)
+let device_already_renamed =
+  let dir = "/etc/sysconfig/network-scripts/interface-rename-data" in
+  Sys.file_exists dir && Sys.is_directory dir
+
+(* If devices have already been renamed, then interface_order is None,
+   since the order is now reflected in their names. *)
+let initial_interface_order = if device_already_renamed then None else Some []
+
+let empty_config =
+  {default_config with interface_order= initial_interface_order}
 
 let config_file_path = "/var/lib/xcp/networkd.db"
 
@@ -148,7 +159,7 @@ let read_management_conf interface_order =
               temp_vlan
           in
           debug "No management bridge in inventory file... using %s" bridge ;
-          if not Network_utils.device_already_renamed then
+          if not device_already_renamed then
             write_manage_iface_to_inventory bridge ;
           bridge
       | Some bridge ->

ocaml/networkd/lib/network_utils.ml

@@ -147,12 +147,6 @@ let fork_script ?on_error ?log script args =
   in
   check_n_run ?on_error ?log fork_script_internal script args
 
-(* If the interface-rename script dir exists, the devices are already renamed
-   to eth<N>, the <N> indicates device order *)
-let device_already_renamed =
-  let dir = "/etc/sysconfig/network-scripts/interface-rename-data" in
-  Sys.file_exists dir && Sys.is_directory dir
-
 module Sysfs = struct
   let list_pci_drivers () =
     try Array.to_list (Sys.readdir "/sys/bus/pci/drivers")