-
Notifications
You must be signed in to change notification settings - Fork 291
CP-54444: Return MAC addresses to host installer #6466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
minglumlu
wants to merge
4
commits into
xapi-project:feature/host-network-device-ordering
Choose a base branch
from
minglumlu:private/mingl/CP-54444
base: feature/host-network-device-ordering
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
529c7f1
CP-54444: Return MAC addresses to host installer
minglumlu 6c56907
fixup! CP-54444: Return MAC addresses to host installer
minglumlu 470d6d1
fixup! fixup! CP-54444: Return MAC addresses to host installer
minglumlu 2679dc3
fixup! fixup! fixup! CP-54444: Return MAC addresses to host installer
minglumlu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,10 @@ open Network_interface | |
|
||
let name = "networkd_db" | ||
|
||
type error = Skip | Msg of string | ||
|
||
let ( let* ) = Result.bind | ||
|
||
let _ = | ||
let bridge = ref "" in | ||
let iface = ref "" in | ||
|
@@ -31,22 +35,62 @@ let _ = | |
(Printf.sprintf "Usage: %s [-bridge <bridge> | -iface <interface>]" name) ; | ||
try | ||
let config = Network_config.read_config () in | ||
if !bridge <> "" then | ||
if List.mem_assoc !bridge config.bridge_config then ( | ||
let bridge_config = List.assoc !bridge config.bridge_config in | ||
let ifaces = | ||
List.concat_map (fun (_, port) -> port.interfaces) bridge_config.ports | ||
in | ||
Printf.printf "interfaces=%s\n" (String.concat "," ifaces) ; | ||
match bridge_config.vlan with | ||
let r = | ||
let* bridge = if !bridge = "" then Error Skip else Ok !bridge in | ||
let* bridge_config = | ||
match List.assoc_opt bridge config.bridge_config with | ||
| Some bridge_config -> | ||
Ok bridge_config | ||
| None -> | ||
() | ||
| Some (parent, id) -> | ||
Printf.printf "vlan=%d\nparent=%s\n" id parent | ||
) else ( | ||
Error (Msg (Printf.sprintf "Could not find bridge %s\n" bridge)) | ||
in | ||
let ifaces = | ||
List.concat_map (fun (_, port) -> port.interfaces) bridge_config.ports | ||
in | ||
let* macs = | ||
let to_mac ~order name = | ||
match List.find_opt (fun dev -> dev.name = name) order with | ||
| Some dev -> | ||
Either.Left (Macaddr.to_string dev.mac) | ||
| None -> | ||
Either.Right name | ||
in | ||
match (config.interface_order, ifaces) with | ||
| Some order, _ :: _ -> | ||
let oks, errs = List.partition_map (to_mac ~order) ifaces in | ||
if errs = [] then | ||
Ok oks | ||
else | ||
Error | ||
(Msg | ||
(Printf.sprintf "Could not find MAC address(es) for %s" | ||
(String.concat ", " errs) | ||
) | ||
) | ||
| _, [] -> | ||
(* No ifaces, no hwaddrs. *) | ||
Ok [] | ||
| None, _ :: _ -> | ||
(* Fallback to use the bridge MAC address when the interface_order | ||
is not available. This can work only because the host installer | ||
requires only one network interface to setup its own networking so far. *) | ||
Ok (Option.to_list bridge_config.bridge_mac) | ||
in | ||
Printf.printf "interfaces=%s\n" (String.concat "," ifaces) ; | ||
Printf.printf "hwaddrs=%s\n" (String.concat "," macs) ; | ||
match bridge_config.vlan with | ||
| None -> | ||
Ok () | ||
| Some (parent, id) -> | ||
Ok (Printf.printf "vlan=%d\nparent=%s\n" id parent) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like using a side effect in the value position of |
||
in | ||
( match r with | ||
| Ok () | Error Skip -> | ||
() | ||
| Error (Msg msg) -> | ||
rc := 1 ; | ||
Printf.fprintf stderr "Could not find bridge %s\n" !bridge | ||
) ; | ||
Printf.fprintf stderr "%s" msg | ||
) ; | ||
if !iface <> "" then | ||
if List.mem_assoc !iface config.interface_config then | ||
let interface_config = List.assoc !iface config.interface_config in | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could use
|> Option.to_result ~none:error
instead of the match, although that will construct the error every single time. It's a weak part of the API. It'd be nicer if it had beenval to_error : none:(unit -> 'e) -> 'a option -> ('a, 'e) result