This is an automated email from the ASF dual-hosted git repository.
bmahler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
The following commit(s) were added to refs/heads/master by this push:
new fd38b0b35 [routing] Change link::setMAC to return Try<Nothing>.
fd38b0b35 is described below
commit fd38b0b351b4e95135d6208f58c81902dbd9ce7f
Author: Jason Zhou <[email protected]>
AuthorDate: Tue Jun 18 16:29:32 2024 -0400
[routing] Change link::setMAC to return Try<Nothing>.
We have noticed that our code does not treat the setMAC bool return value
differently based on whether it returns true or false. As such, we are
changing the return type to return Nothing so that we either return Error
or Nothing, rather than Error or True or False.
As a consequence of this we are also removing the special case of returning
False but not Error when we get ENODEV from ioctl.
Review: https://reviews.apache.org/r/75056/
---
src/linux/routing/link/link.cpp | 30 ++++++++--------------
src/linux/routing/link/link.hpp | 2 +-
.../mesos/isolators/network/port_mapping.cpp | 4 +--
src/tests/containerizer/routing_tests.cpp | 6 ++---
4 files changed, 16 insertions(+), 26 deletions(-)
diff --git a/src/linux/routing/link/link.cpp b/src/linux/routing/link/link.cpp
index bff172dea..87ca26c54 100644
--- a/src/linux/routing/link/link.cpp
+++ b/src/linux/routing/link/link.cpp
@@ -246,7 +246,7 @@ Try<bool> setUp(const string& link)
}
-Try<bool> setMAC(const string& link, const net::MAC& mac)
+Try<Nothing> setMAC(const string& link, const net::MAC& mac)
{
// TODO(jieyu): We use ioctl to set the MAC address because the
// interfaces in libnl have some issues with virtual devices.
@@ -264,15 +264,10 @@ Try<bool> setMAC(const string& link, const net::MAC& mac)
// to get the MAC address of the link first to decide what value the
// sa_family should be.
if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
- if (errno == ENODEV) {
- os::close(fd);
- return false;
- } else {
- // Save the error string as os::close may overwrite errno.
- const string message = os::strerror(errno);
- os::close(fd);
- return Error(message);
- }
+ // Save the error string as os::close may overwrite errno.
+ const string message = os::strerror(errno);
+ os::close(fd);
+ return Error(message);
}
ifr.ifr_hwaddr.sa_data[0] = mac[0];
@@ -283,19 +278,14 @@ Try<bool> setMAC(const string& link, const net::MAC& mac)
ifr.ifr_hwaddr.sa_data[5] = mac[5];
if (ioctl(fd, SIOCSIFHWADDR, &ifr) == -1) {
- if (errno == ENODEV) {
- os::close(fd);
- return false;
- } else {
- // Save the error string as os::close may overwrite errno.
- const string message = os::strerror(errno);
- os::close(fd);
- return Error(message);
- }
+ // Save the error string as os::close may overwrite errno.
+ const string message = os::strerror(errno);
+ os::close(fd);
+ return Error(message);
}
os::close(fd);
- return true;
+ return Nothing();
}
diff --git a/src/linux/routing/link/link.hpp b/src/linux/routing/link/link.hpp
index 35639afa4..09790f317 100644
--- a/src/linux/routing/link/link.hpp
+++ b/src/linux/routing/link/link.hpp
@@ -82,7 +82,7 @@ Try<bool> setUp(const std::string& link);
// Sets the MAC address of the link. Returns false if the link is not
// found.
-Try<bool> setMAC(const std::string& link, const net::MAC& mac);
+Try<Nothing> setMAC(const std::string& link, const net::MAC& mac);
// Returns the Maximum Transmission Unit (MTU) of the link. Returns
diff --git a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
index 1fd2e226c..3b3b899fb 100644
--- a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
+++ b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
@@ -2504,7 +2504,7 @@ Try<Isolator*> PortMappingIsolatorProcess::create(const
Flags& flags)
// recent kernel patch is needed for this operation to succeed:
// https://git.kernel.org/cgit/linux/kernel/git/davem/net.git/:
// 25f929fbff0d1bcebf2e92656d33025cd330cbf8
- Try<bool> setHostLoMAC = link::setMAC(lo.get(), hostMAC.get());
+ Try<Nothing> setHostLoMAC = link::setMAC(lo.get(), hostMAC.get());
if (setHostLoMAC.isError()) {
return Error(
"Failed to set the MAC address of " + lo.get() +
@@ -3625,7 +3625,7 @@ Future<Nothing> PortMappingIsolatorProcess::isolate(
// Sets the MAC address of veth to match the MAC address of the host
// public interface (eth0).
- Try<bool> setVethMAC = link::setMAC(veth(pid), hostMAC);
+ Try<Nothing> setVethMAC = link::setMAC(veth(pid), hostMAC);
if (setVethMAC.isError()) {
return Failure(
"Failed to set the MAC address of " + veth(pid) +
diff --git a/src/tests/containerizer/routing_tests.cpp
b/src/tests/containerizer/routing_tests.cpp
index a030668cb..c9fa2e86a 100644
--- a/src/tests/containerizer/routing_tests.cpp
+++ b/src/tests/containerizer/routing_tests.cpp
@@ -376,8 +376,8 @@ TEST_F(RoutingVethTest, ROOT_LinkSetMAC)
uint8_t bytes[6] = {0x12, 0x34, 0x56, 0x78, 0x9a, 0xbc};
- EXPECT_SOME_TRUE(link::setMAC(TEST_VETH_LINK, net::MAC(bytes)));
- EXPECT_SOME_TRUE(link::setMAC(TEST_PEER_LINK, net::MAC(bytes)));
+ EXPECT_SOME(link::setMAC(TEST_VETH_LINK, net::MAC(bytes)));
+ EXPECT_SOME(link::setMAC(TEST_PEER_LINK, net::MAC(bytes)));
Result<net::MAC> mac = net::mac(TEST_VETH_LINK);
@@ -389,7 +389,7 @@ TEST_F(RoutingVethTest, ROOT_LinkSetMAC)
ASSERT_SOME(mac);
EXPECT_EQ(mac.get(), net::MAC(bytes));
- EXPECT_SOME_FALSE(link::setMAC("non-exist", net::MAC(bytes)));
+ EXPECT_ERROR(link::setMAC("non-exist", net::MAC(bytes)));
// Kernel will reject a multicast MAC address.
uint8_t multicast[6] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
