CP-54444: Return MAC addresses to host installer #6466
Conversation
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
The host installer uses this utility to get the management interface from the management bridge. Now it changes to use MAC address(es) to find out the management interface(s). This is because the interface-rename functionality will be deprecated and the names of the network interfaces are not guaranteed to be the same between dom0 and host installer's running environment. Note that this change must be delivered to a host before upgrading to a new version in which the interface-rename is deprecated because the host installer is built from the new version and it will not be able to find the management network interface by name if the networkd_db command returns only names generated by interface-rename. Specifically, the "interface_order" field is only available when the networkd takes place of interface-rename to generate order. Before that, only the "bridge_mac" can be used because at that time, the host installer only uses one interface to setup its own networking during installation and no MAC addresses are recorded in networkd.db for individual interfaces. The "bridge_mac" is just the MAC address of one of the interfaces which construct the management bridge. Signed-off-by: Ming Lu <[email protected]>
|
xenserver/host-installer#249 is for the host installer change. |
changlei-li
reviewed
May 15, 2025
psafont
reviewed
May 15, 2025
ocaml/networkd/bin_db/networkd_db.ml
Outdated
Comment on lines
35
to
36
| if List.mem_assoc !bridge config.bridge_config then | ||
| let bridge_config = List.assoc !bridge config.bridge_config in |
There was a problem hiding this comment.
Suggested change
| if List.mem_assoc !bridge config.bridge_config then | |
| let bridge_config = List.assoc !bridge config.bridge_config in | |
| match List.assoc_opt !bridge config.bridge_config with | |
| | Some bridge_config -> |
psafont
reviewed
May 15, 2025
ocaml/networkd/bin_db/networkd_db.ml
Outdated
| Error (Printf.sprintf "Could not find MAC address of %s\n" name) | ||
| in | ||
| let macs = | ||
| match config.interface_order with |
There was a problem hiding this comment.
I would rather have not use the if for ifaces and incorporate it to the match
Suggested change
| match config.interface_order with | |
| match (config.interface_order, ifaces) with |
psafont
reviewed
May 15, 2025
psafont
reviewed
May 15, 2025
psafont
reviewed
May 15, 2025
ocaml/networkd/bin_db/networkd_db.ml
Outdated
| Printf.fprintf stderr "%s\n" msg | ||
| | Ok macs -> ( | ||
| Printf.printf "interfaces=%s\n" (String.concat "," ifaces) ; | ||
| Printf.printf "hwaddrs=%s\n" (String.concat "," (List.rev macs)) ; |
There was a problem hiding this comment.
Does adding this line work with the current script?
There was a problem hiding this comment.
Yes. The current script doesn't care about any extra keys in the output.
Signed-off-by: Ming Lu <[email protected]>
changlei-li
approved these changes
May 16, 2025
psafont
reviewed
May 16, 2025
Signed-off-by: Ming Lu <[email protected]>
lindig
reviewed
May 16, 2025
| in | ||
| Printf.printf "interfaces=%s\n" (String.concat "," ifaces) ; | ||
| match bridge_config.vlan with | ||
| ( if !bridge <> "" then |
There was a problem hiding this comment.
Could the case of !bridge = "" not be handled as part of List.assoc_opt returning Null? I would try to reduce the deep nesting of conditions.
There was a problem hiding this comment.
This is to distinguish the cases that no -bridge option and -bridge <br> where <br> is an invalid one.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The host installer uses this utility to get the management interface from the management bridge. Now it changes to use MAC address(es) to find out the management interface(s). This is because the interface-rename functionality will be deprecated and the names of the network interfaces are not guaranteed to be the same between dom0 and host installer's running environment.
Note that this change must be delivered to a host before upgrading to a new version in which the interface-rename is deprecated because the host installer is built from the new version and it will not be able to find the management network interface by name if the networkd_db command returns only names generated by interface-rename.
Specifically, the "interface_order" field is only available when the networkd takes place of interface-rename to generate order. Before that, only the "bridge_mac" can be used because at that time, the host installer only uses one interface to setup its own networking during installation and no MAC addresses are recorded in networkd.db for individual interfaces. The "bridge_mac" is just the MAC address of one of the interfaces which construct the management bridge.