Copilot commented on code in PR #14060:
URL: https://github.com/apache/cloudstack/pull/14060#discussion_r4518136969
##########
utils/src/main/java/com/cloud/utils/net/NetUtils.java:
##########
@@ -306,7 +316,25 @@ public static String getDefaultEthDevice() {
final String defDev = Script.runSimpleBashScript("/sbin/route -n
get default 2> /dev/null | grep interface | awk '{print $2}'");
return defDev;
}
- return Script.runSimpleBashScript("ip route show default 0.0.0.0/0 |
head -1 | awk '{print $5}'");
+ final String defaultIp4Device = getDefaultEthDevice(false);
+ if (defaultIp4Device != null) {
+ return defaultIp4Device;
+ }
+ LOGGER.debug("No IPv4 default route found, falling back to the IPv6
default route to determine the default network device.");
+ return getDefaultEthDevice(true);
Review Comment:
`getDefaultEthDevice()` only falls back to IPv6 when the IPv4 result is
`null`. If the underlying script returns an empty/blank string (e.g., awk
prints nothing), this method will return a blank device name and *skip* the
IPv6 fallback, causing false “no default NIC” behavior. Normalize the script
output (trim) and treat blank as “not found” (return `null`) before the
`defaultIp4Device != null` check.
##########
utils/src/main/java/com/cloud/utils/net/NetUtils.java:
##########
@@ -306,7 +316,25 @@ public static String getDefaultEthDevice() {
final String defDev = Script.runSimpleBashScript("/sbin/route -n
get default 2> /dev/null | grep interface | awk '{print $2}'");
return defDev;
}
- return Script.runSimpleBashScript("ip route show default 0.0.0.0/0 |
head -1 | awk '{print $5}'");
+ final String defaultIp4Device = getDefaultEthDevice(false);
+ if (defaultIp4Device != null) {
+ return defaultIp4Device;
+ }
+ LOGGER.debug("No IPv4 default route found, falling back to the IPv6
default route to determine the default network device.");
+ return getDefaultEthDevice(true);
+ }
+
+ /**
+ * Returns the name of the network device used by the IPv4 or IPv6 default
route, or null when there is no such
+ * default route.
+ *
+ * An IPv4 default route is not a requirement, a host can be IPv6-only or
have IPv4 connectivity without a default
+ * route, therefore both address families are looked up separately.
+ */
+ protected static String getDefaultEthDevice(final boolean ipv6) {
Review Comment:
This introduces a new `protected` static method on a utility class, which
unintentionally expands the API surface (accessible to subclasses and within
the package) without a clear need. If the intent is only internal
reuse/testability, prefer `private` or package-private visibility to limit
exposure and reduce future compatibility obligations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
