Fix review comment

- function is_sorted_by_script to val device_renamed_already
- reduce nested structure in update config
- handle port name update
- bring back read inventory to get bridge name

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

Author: changlei-li

ocaml/networkd/bin/network_server.ml

@@ -36,7 +36,7 @@ let write_config () =
     with Network_config.Write_error -> ()
 
 let sort_and_update () =
-  if Network_utils.is_sorted_by_script () then
+  if Network_utils.device_already_renamed then
     ()
   else
     let last_config = !config in
@@ -45,48 +45,29 @@ let sort_and_update () =
     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
-                      (fun (port, port_conf) ->
-                        ( port
-                        , {
-                            port_conf with
-                            interfaces=
-                              List.map
-                                (fun iface ->
-                                  match
-                                    List.assoc_opt iface changed_interfaces
-                                  with
-                                  | Some new_name ->
-                                      new_name
-                                  | None ->
-                                      iface
-                                )
-                                port_conf.interfaces
-                          }
-                        )
-                      )
-                      bridge_conf.ports
-                }
+              , {bridge_conf with ports= List.map update_port bridge_conf.ports}
               )
             )
             last_config.bridge_config
         in
         let interface_config =
           List.map
-            (fun (name, conf) ->
-              match List.assoc_opt name changed_interfaces with
-              | Some new_name ->
-                  (new_name, conf)
-              | None ->
-                  (name, conf)
-            )
+            (fun (name, conf) -> (update_name name, conf))
             last_config.interface_config
         in
         config :=
@@ -101,7 +82,7 @@ let sort_and_update () =
           (Network_device_order.string_of_error err)
 
 let sort_on_first_boot () =
-  if Network_utils.is_sorted_by_script () then
+  if Network_utils.device_already_renamed then
     None
   else
     match Network_device_order.sort [] with
@@ -371,7 +352,7 @@ module Interface = struct
   let get_interface_positions dbg () =
     Debug.with_thread_associated dbg
       (fun () ->
-        if Network_utils.is_sorted_by_script () then
+        if Network_utils.device_already_renamed then
           sort_based_on_ethx ()
         else
           Option.value ~default:[] !config.interface_order

ocaml/networkd/lib/network_config.ml

@@ -132,19 +132,28 @@ let read_management_conf interface_order =
         order
     in
     let bridge_name =
-      let bridge =
-        if vlan = None then
-          bridge_naming_convention device pos_opt
-        else
-          (* At this point, we don't know what the VLAN bridge name will be,
-             * so use a temporary name. Xapi will replace the bridge once the name
-             * has been decided on. *)
-          temp_vlan
+      let inventory_bridge =
+        try Some (Inventory.lookup Inventory._management_interface)
+        with Inventory.Missing_inventory_key _ -> None
       in
-      debug "Management bridge name %s" bridge ;
-      if not @@ Network_utils.is_sorted_by_script () then
-        write_manage_iface_to_inventory bridge ;
-      bridge
+      match inventory_bridge with
+      | Some "" | None ->
+          let bridge =
+            if vlan = None then
+              bridge_naming_convention device pos_opt
+            else
+              (* At this point, we don't know what the VLAN bridge name will be,
+                 * so use a temporary name. Xapi will replace the bridge once the name
+                 * has been decided on. *)
+              temp_vlan
+          in
+          debug "No management bridge in inventory file... using %s" bridge ;
+          if not Network_utils.device_already_renamed then
+            write_manage_iface_to_inventory bridge ;
+          bridge
+      | Some bridge ->
+          debug "Management bridge in inventory file: %s" bridge ;
+          bridge
     in
     let mac = Network_utils.Ip.get_mac device in
     let dns = parse_dns_config args in

ocaml/networkd/lib/network_utils.ml

@@ -147,7 +147,9 @@ let fork_script ?on_error ?log script args =
   in
   check_n_run ?on_error ?log fork_script_internal script args
 
-let is_sorted_by_script () =
+(* 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