Create and use bridge

include/multipass/network_interface_info.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2020 Canonical, Ltd.
+ * Copyright (C) 2020-2021 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -21,13 +21,24 @@
 #include <multipass/ip_address.h>
 #include <multipass/optional.h>
 
+#include <algorithm>
+#include <string>
+#include <vector>
+
 namespace multipass
 {
 struct NetworkInterfaceInfo
 {
+    bool has_link(const std::string& net) const
+    {
+        auto end = links.cend();
+        return std::find(links.cbegin(), end, net) != end;
+    }
+
     std::string id;
     std::string type;
     std::string description;
+    std::vector<std::string> links = {}; // default initializer allows aggregate init of the other 3
 };
 } // namespace multipass
 #endif // MULTIPASS_NETWORK_INTERFACE_INFO_H

include/multipass/virtual_machine_factory.h

@@ -36,6 +36,7 @@ class URLDownloader;
 class VirtualMachineDescription;
 class VMImageHost;
 class VMStatusMonitor;
+struct NetworkInterface;
 struct NetworkInterfaceInfo;
 
 class VirtualMachineFactory
@@ -53,6 +54,7 @@ class VirtualMachineFactory
     virtual void remove_resources_for(const std::string& name) = 0;
 
     virtual FetchType fetch_type() = 0;
+    virtual void prepare_networking(std::vector<NetworkInterface>& extra_interfaces) = 0; // note the arg may be updated
     virtual VMImage prepare_source_image(const VMImage& source_image) = 0;
     virtual void prepare_instance_image(const VMImage& instance_image, const VirtualMachineDescription& desc) = 0;
     virtual void hypervisor_health_check() = 0;

src/daemon/daemon.cpp

@@ -2141,6 +2141,8 @@ void mp::Daemon::create_vm(const CreateRequest* request, grpc::ServerWriter<Crea
             reply.set_create_message("Configuring " + name);
             server->Write(reply);
 
+            config->factory->prepare_networking(checked_args.extra_interfaces);
+
             // This set stores the MAC's which need to be in the allocated_mac_addrs if everything goes well.
             auto new_macs = allocated_mac_addrs;
 

src/platform/backends/lxd/lxd_virtual_machine_factory.cpp

@@ -19,9 +19,12 @@
 #include "lxd_virtual_machine.h"
 #include "lxd_vm_image_vault.h"
 
+#include <shared/linux/backend_utils.h>
+
 #include <multipass/exceptions/local_socket_connection_exception.h>
 #include <multipass/format.h>
 #include <multipass/logging/log.h>
+#include <multipass/network_interface.h>
 #include <multipass/network_interface_info.h>
 #include <multipass/platform.h>
 #include <multipass/utils.h>
@@ -36,6 +39,7 @@ namespace
 {
 constexpr auto category = "lxd factory";
 const QString multipass_bridge_name = "mpbr0";
+
 } // namespace
 
 mp::LXDVirtualMachineFactory::LXDVirtualMachineFactory(NetworkAccessManager::UPtr manager, const mp::Path& data_dir,
@@ -173,7 +177,8 @@ auto mp::LXDVirtualMachineFactory::networks() const -> std::vector<NetworkInterf
                         auto description = lxd_description.isEmpty() ? std::move(platform_it->second.description)
                                                                      : lxd_description.toStdString();
 
-                        ret.push_back({std::move(id), std::move(type), std::move(description)});
+                        ret.push_back({std::move(id), std::move(type), std::move(description),
+                                       std::move(platform_it->second.links)});
 
                         platform_networks.erase(platform_it); // prevent matching with this network again
                     }
@@ -184,3 +189,20 @@ auto mp::LXDVirtualMachineFactory::networks() const -> std::vector<NetworkInterf
 
     return ret;
 }
+
+void mp::LXDVirtualMachineFactory::prepare_networking(std::vector<NetworkInterface>& extra_interfaces)
+{
+    auto host_nets = networks();
+    for (auto& net : extra_interfaces)
+    {
+        auto it = std::find_if(host_nets.cbegin(), host_nets.cend(),
+                               [&net](const mp::NetworkInterfaceInfo& info) { return info.id == net.id; });
+        if (it != host_nets.end() && it->type == "ethernet")
+        {
+            it = std::find_if(host_nets.cbegin(), host_nets.cend(), [&net](const mp::NetworkInterfaceInfo& info) {
+                return info.type == "bridge" && info.has_link(net.id);
+            });
+            net.id = it != host_nets.cend() ? it->id : mp::backend::create_bridge_with(net.id);
+        }
+    }
+}

src/platform/backends/lxd/lxd_virtual_machine_factory.h

@@ -34,6 +34,7 @@ class LXDVirtualMachineFactory final : public BaseVirtualMachineFactory
     explicit LXDVirtualMachineFactory(NetworkAccessManager::UPtr manager, const Path& data_dir,
                                       const QUrl& base_url = lxd_socket_url);
 
+    void prepare_networking(std::vector<NetworkInterface>& extra_interfaces) override;
     VirtualMachine::UPtr create_virtual_machine(const VirtualMachineDescription& desc,
                                                 VMStatusMonitor& monitor) override;
     void remove_resources_for(const std::string& name) override;

src/platform/backends/shared/base_virtual_machine_factory.h

@@ -44,6 +44,11 @@ class BaseVirtualMachineFactory : public VirtualMachineFactory
         return {};
     };
 
+    void prepare_networking(std::vector<NetworkInterface>& /*extra_interfaces*/) override
+    {
+        // only certain backends need to do anything to prepare networking
+    }
+
     VMImageVault::UPtr create_image_vault(std::vector<VMImageHost*> image_hosts, URLDownloader* downloader,
                                           const Path& cache_dir_path, const Path& data_dir_path,
                                           const days& days_to_expire) override

src/platform/backends/shared/linux/backend_utils.cpp

@@ -322,7 +322,7 @@ void mp::backend::check_if_kvm_is_in_use()
 
 // @precondition no bridge exists for this interface
 // @precondition interface identifies an ethernet device
-void mp::backend::create_bridge_with(const std::string& interface)
+std::string mp::backend::create_bridge_with(const std::string& interface)
 {
     static constexpr auto log_category_create = "create bridge";
     static constexpr auto log_category_rollback = "rollback bridge";
@@ -359,7 +359,11 @@ void mp::backend::create_bridge_with(const std::string& interface)
     for '/' to signal null `device` and `specific-object` derived from nmcli and libnm. See https://bit.ly/3dMA3QB */
 
     rollback_guard.dismiss(); // we succeeded!
-    mpl::log(mpl::Level::info, log_category_create, fmt::format("Created bridge: {}", parent_name));
+
+    auto ret = parent_name.toStdString();
+    mpl::log(mpl::Level::info, log_category_create, fmt::format("Created bridge: {}", ret));
+
+    return ret;
 }
 
 mp::backend::CreateBridgeException::CreateBridgeException(const std::string& detail, const QDBusError& dbus_error,

src/platform/backends/shared/linux/backend_utils.h

@@ -40,7 +40,7 @@ Path convert_to_qcow_if_necessary(const Path& image_path);
 QString cpu_arch();
 void check_for_kvm_support();
 void check_if_kvm_is_in_use();
-void create_bridge_with(const std::string& interface);
+std::string create_bridge_with(const std::string& interface);
 
 class CreateBridgeException : public std::runtime_error
 {

src/platform/platform_linux.cpp

@@ -110,25 +110,47 @@ bool is_ethernet(const QDir& net_dir)
            get_net_devtype(net_dir).isEmpty();
 }
 
-mp::NetworkInterfaceInfo get_network(const QDir& net_dir)
+mp::optional<mp::NetworkInterfaceInfo> get_network(const QDir& net_dir)
 {
     static const auto bridge_fname = QStringLiteral("brif");
+    auto id = net_dir.dirName().toStdString();
 
-    std::string type, description;
     if (auto bridge = "bridge"; net_dir.exists(bridge))
     {
-        type = bridge;
+        std::vector<std::string> links;
         QStringList bridge_members = QDir{net_dir.filePath(bridge_fname)}.entryList(QDir::NoDotAndDotDot | QDir::Dirs);
-        description = bridge_members.isEmpty() ? "Empty network bridge"
-                                               : fmt::format("Network bridge with {}", bridge_members.join(", "));
+
+        links.reserve(bridge_members.size());
+        std::transform(bridge_members.cbegin(), bridge_members.cend(), std::back_inserter(links),
+                       [](const QString& interface) { return interface.toStdString(); });
+
+        return {{std::move(id), bridge, /*description=*/"", std::move(links)}}; // description needs updating with links
     }
     else if (is_ethernet(net_dir))
+        return {{std::move(id), "ethernet", "Ethernet device"}};
+
+    return mp::nullopt;
+}
+
+void update_bridges(std::map<std::string, mp::NetworkInterfaceInfo>& networks)
+{
+    for (auto& item : networks)
     {
-        type = "ethernet";
-        description = "Ethernet device";
+        if (auto& net = item.second; net.type == "bridge")
+        { // bridge descriptions and links depend on what other networks we recognized
+            auto& links = net.links;
+            auto is_unknown = [&networks](const std::string& id) {
+                auto same_as = [&id](const auto& other) { return other.first == id; };
+                return std::find_if(networks.cbegin(), networks.cend(), same_as) == networks.cend();
+            };
+            links.erase(std::remove_if(links.begin(), links.end(), is_unknown),
+                        links.end()); // filter links to networks we don't recognize
+
+            net.description =
+                links.empty() ? "Network bridge"
+                              : fmt::format("Network bridge with {}", fmt::join(links.cbegin(), links.cend(), ", "));
+        }
     }
-
-    return mp::NetworkInterfaceInfo{net_dir.dirName().toStdString(), std::move(type), std::move(description)};
 }
 } // namespace
 
@@ -160,11 +182,15 @@ auto mp::platform::detail::get_network_interfaces_from(const QDir& sys_dir)
     auto ifaces_info = std::map<std::string, mp::NetworkInterfaceInfo>();
     for (const auto& entry : sys_dir.entryList(QDir::NoDotAndDotDot | QDir::Dirs))
     {
-        auto iface = get_network(QDir{sys_dir.filePath(entry)});
-        auto name = iface.id; // (can't rely on param evaluation order)
-        ifaces_info.emplace(std::move(name), std::move(iface));
+        if (auto iface = get_network(QDir{sys_dir.filePath(entry)}); iface)
+        {
+            auto name = iface->id; // (can't rely on param evaluation order)
+            ifaces_info.emplace(std::move(name), std::move(*iface));
+        }
     }
 
+    update_bridges(ifaces_info);
+
     return ifaces_info;
 }
 

tests/linux/test_backend_utils.cpp

@@ -312,7 +312,7 @@ struct CreateBridgeTest : public Test
             QMap<QString, QVariantMap> outer_map;
             QMap<QString, QVariantMap>::const_iterator outer_it;
             QVariantMap::const_iterator inner_it;
-            QString parent_name = get_bridge_name(child).left(15);
+            QString parent_name = get_bridge_name(child);
 
             return (outer_map = arg.value<QMap<QString, QVariantMap>>()).size() == 2 &&
                    (outer_it = outer_map.find("connection")) != outer_map.end() && outer_it->size() == 3 &&
@@ -330,7 +330,7 @@ struct CreateBridgeTest : public Test
             QMap<QString, QVariantMap> outer_map;
             QMap<QString, QVariantMap>::const_iterator outer_it;
             QVariantMap::const_iterator inner_it;
-            QString parent_name = get_bridge_name(child).left(15);
+            QString parent_name = get_bridge_name(child);
             QString child_name = parent_name + "-child";
 
             return (outer_map = arg.value<QMap<QString, QVariantMap>>()).size() == 1 &&
@@ -350,6 +350,11 @@ struct CreateBridgeTest : public Test
                         Property(&QDBusObjectPath::path, Property(&QString::toStdString, Eq(path))));
     }
 
+    static QString get_bridge_name(const char* child)
+    {
+        return (QString{"br-"} + child).left(15);
+    }
+
     MockDBusProvider::GuardedMock mock_dbus_injection = MockDBusProvider::inject();
     MockDBusProvider* mock_dbus_provider = mock_dbus_injection.first;
     MockDBusConnection mock_bus{};
@@ -358,17 +363,11 @@ struct CreateBridgeTest : public Test
     mpt::MockLogger::Scope logger_scope = mpt::MockLogger::inject();
 
     const QVariant empty{};
-
-private:
-    static QString get_bridge_name(const char* child)
-    {
-        return QString{"br-"} + child;
-    }
 };
 
 TEST_F(CreateBridgeTest, creates_and_activates_connections) // success case
 {
-    static constexpr auto network = "wlan1234567890a";
+    static constexpr auto network = "eth1234567890a";
     static constexpr auto child_obj_path = "/an/obj/path/for/child";
     static constexpr auto null_obj_path = "/";
 
@@ -390,7 +389,7 @@ TEST_F(CreateBridgeTest, creates_and_activates_connections) // success case
     }
 
     inject_dbus_interfaces();
-    EXPECT_NO_THROW(mp::backend::create_bridge_with(network));
+    EXPECT_EQ(mp::backend::create_bridge_with(network), get_bridge_name(network).toStdString());
 }
 
 TEST_F(CreateBridgeTest, throws_if_bus_disconnected)