Consolidate VM fields

include/multipass/ip_address.h

@@ -27,13 +27,12 @@ namespace multipass
 {
 struct IPAddress
 {
-    IPAddress(std::array<uint8_t, 4> octets);
-    IPAddress(const std::string& ip_string);
+    explicit IPAddress(std::array<uint8_t, 4> octets);
+    explicit IPAddress(const std::string& ip_string);
     explicit IPAddress(uint32_t value);
-    IPAddress(const IPAddress& other) = default;
 
-    std::string as_string() const;
-    uint32_t as_uint32() const;
+    [[nodiscard]] std::string as_string() const;
+    [[nodiscard]] uint32_t as_uint32() const;
 
     bool operator==(const IPAddress& other) const;
     bool operator!=(const IPAddress& other) const;

include/multipass/virtual_machine.h

@@ -18,9 +18,7 @@
 #pragma once
 
 #include "disabled_copy_move.h"
-#include "ip_address.h"
 #include "network_interface.h"
-#include "path.h"
 
 #include <QDir>
 #include <QJsonObject>
@@ -29,12 +27,12 @@
 #include <condition_variable>
 #include <memory>
 #include <mutex>
-#include <optional>
 #include <string>
 #include <vector>
 
 namespace multipass
 {
+struct IPAddress;
 class MemorySize;
 class VMMount;
 struct VMSpecs;
@@ -80,9 +78,8 @@ class VirtualMachine : private DisabledCopyMove
     };
     virtual std::string ssh_hostname(std::chrono::milliseconds timeout) = 0;
     virtual std::string ssh_username() = 0;
-    virtual std::string management_ipv4() = 0;
-    virtual std::vector<std::string> get_all_ipv4() = 0;
-    virtual std::string ipv6() = 0;
+    virtual std::optional<IPAddress> management_ipv4() = 0;
+    virtual std::vector<IPAddress> get_all_ipv4() = 0;
 
     // careful: default param in virtual method; be sure to keep the same value in all descendants
     virtual std::string ssh_exec(const std::string& cmd, bool whisper = false) = 0;
@@ -122,28 +119,20 @@ class VirtualMachine : private DisabledCopyMove
     virtual std::vector<std::string> get_childrens_names(const Snapshot* parent) const = 0;
     virtual int get_snapshot_count() const = 0;
 
-    QDir instance_directory() const;
+    virtual QDir instance_directory() const = 0;
 
     VirtualMachine::State state;
     const std::string vm_name;
     std::condition_variable state_wait;
     std::mutex state_mutex;
-    std::optional<IPAddress> management_ip;
-    bool shutdown_while_starting{false};
 
 protected:
-    const QDir instance_dir;
-
-    VirtualMachine(VirtualMachine::State state,
-                   const std::string& vm_name,
-                   const Path& instance_dir)
-        : state{state}, vm_name{vm_name}, instance_dir{QDir{instance_dir}} {};
-    VirtualMachine(const std::string& vm_name, const Path& instance_dir)
-        : VirtualMachine(State::off, vm_name, instance_dir){};
+    explicit VirtualMachine(const std::string& vm_name) : VirtualMachine(State::off, vm_name)
+    {
+    }
+
+    VirtualMachine(State state, const std::string& vm_name) : state{state}, vm_name{vm_name}
+    {
+    }
 };
 } // namespace multipass
-
-inline QDir multipass::VirtualMachine::instance_directory() const
-{
-    return instance_dir; // TODO this should probably only be known at the level of the base VM
-}

src/client/cli/formatter/csv_formatter.cpp

@@ -86,7 +86,7 @@ std::string generate_instance_details(const mp::InfoReply reply)
 
     fmt::memory_buffer buf;
     fmt::format_to(std::back_inserter(buf),
-                   "Name,State,Ipv4,Ipv6,Release,Image hash,Image release,Load,Disk usage,Disk "
+                   "Name,State,Ipv4,Release,Image hash,Image release,Load,Disk usage,Disk "
                    "total,Memory usage,Memory "
                    "total,Mounts,AllIPv4,CPU(s){}\n",
                    have_num_snapshots ? ",Snapshots" : "");
@@ -96,11 +96,10 @@ std::string generate_instance_details(const mp::InfoReply reply)
         const auto& instance_details = info.instance_info();
 
         fmt::format_to(std::back_inserter(buf),
-                       "{},{},{},{},{},{},{},{},{},{},{},{},{},{},{}{}\n",
+                       "{},{},{},{},{},{},{},{},{},{},{},{},{},{}{}\n",
                        info.name(),
                        mp::format::status_string_for(info.instance_status()),
                        instance_details.ipv4_size() ? instance_details.ipv4(0) : "",
-                       instance_details.ipv6_size() ? instance_details.ipv6(0) : "",
                        instance_details.current_release(),
                        instance_details.id(),
                        instance_details.image_release(),
@@ -123,16 +122,15 @@ std::string generate_instances_list(const mp::InstancesList& instance_list)
 {
     fmt::memory_buffer buf;
 
-    fmt::format_to(std::back_inserter(buf), "Name,State,IPv4,IPv6,Release,AllIPv4\n");
+    fmt::format_to(std::back_inserter(buf), "Name,State,IPv4,Release,AllIPv4\n");
 
     for (const auto& instance : mp::format::sorted(instance_list.instances()))
     {
         fmt::format_to(std::back_inserter(buf),
-                       "{},{},{},{},{},\"{}\"\n",
+                       "{},{},{},{},\"{}\"\n",
                        instance.name(),
                        mp::format::status_string_for(instance.instance_status()),
                        instance.ipv4_size() ? instance.ipv4(0) : "",
-                       instance.ipv6_size() ? instance.ipv6(0) : "",
                        instance.current_release().empty() ? "Not Available"
                                                           : instance.current_release(),
                        fmt::join(instance.ipv4(), ","));

src/client/cli/formatter/table_formatter.cpp

@@ -148,14 +148,6 @@ void generate_instance_details(Dest&& dest, const mp::DetailedInfoItem& item)
     for (int i = 1; i < ipv4_size; ++i)
         fmt::format_to(dest, "{:<16}{}\n", "", instance_details.ipv4(i));
 
-    if (int ipv6_size = instance_details.ipv6_size())
-    {
-        fmt::format_to(dest, "{:<16}{}\n", "IPv6:", instance_details.ipv6(0));
-
-        for (int i = 1; i < ipv6_size; ++i)
-            fmt::format_to(dest, "{:<16}{}\n", "", instance_details.ipv6(i));
-    }
-
     fmt::format_to(dest,
                    "{:<16}{}\n",
                    "Release:",

src/daemon/daemon.cpp

@@ -1909,17 +1909,15 @@ try
 
         if (request->request_ipv4() && MP_UTILS.is_running(present_state))
         {
-            std::string management_ip = vm.management_ipv4();
+            auto management_ip = vm.management_ipv4();
             auto all_ipv4 = vm.get_all_ipv4();
 
-            if (MP_UTILS.is_ipv4_valid(management_ip))
-                entry->add_ipv4(management_ip);
-            else if (all_ipv4.empty())
-                entry->add_ipv4("N/A");
+            if (management_ip)
+                entry->add_ipv4(management_ip->as_string());
 
             for (const auto& extra_ipv4 : all_ipv4)
                 if (extra_ipv4 != management_ip)
-                    entry->add_ipv4(extra_ipv4);
+                    entry->add_ipv4(extra_ipv4.as_string());
         }
 
         return grpc::Status::OK;

src/daemon/runtime_instance_info_helper.cpp

@@ -140,15 +140,13 @@ void mp::RuntimeInstanceInfoHelper::populate_runtime_info(mp::VirtualMachine& vm
     instance_info->set_current_release(!current_release.empty() ? current_release
                                                                 : original_release);
 
-    std::string management_ip = vm.management_ipv4();
+    auto management_ip = vm.management_ipv4();
     auto all_ipv4 = vm.get_all_ipv4();
 
-    if (MP_UTILS.is_ipv4_valid(management_ip))
-        instance_info->add_ipv4(management_ip);
-    else if (all_ipv4.empty())
-        instance_info->add_ipv4("N/A");
+    if (management_ip)
+        instance_info->add_ipv4(management_ip->as_string());
 
     for (const auto& extra_ipv4 : all_ipv4)
         if (extra_ipv4 != management_ip)
-            instance_info->add_ipv4(extra_ipv4);
+            instance_info->add_ipv4(extra_ipv4.as_string());
 }

src/network/ip_address.cpp

@@ -17,7 +17,11 @@
 
 #include <multipass/ip_address.h>
 
+#include <array>
+#include <cstdint>
 #include <sstream>
+#include <stdexcept>
+#include <string>
 
 namespace mp = multipass;
 
@@ -63,7 +67,7 @@ mp::IPAddress::IPAddress(std::array<uint8_t, 4> octets) : octets{octets}
 {
 }
 
-mp::IPAddress::IPAddress(const std::string& ip) : IPAddress(parse(ip))
+mp::IPAddress::IPAddress(const std::string& ip_string) : IPAddress(parse(ip_string))
 {
 }
 

src/platform/backends/hyperv/hyperv_virtual_machine.cpp

@@ -19,17 +19,16 @@
 #include "hyperv_snapshot.h"
 
 #include <multipass/constants.h>
-#include <multipass/exceptions/not_implemented_on_this_backend_exception.h> // TODO@snapshots drop
 #include <multipass/exceptions/start_exception.h>
 #include <multipass/exceptions/virtual_machine_state_exceptions.h>
+#include <multipass/ip_address.h>
 #include <multipass/logging/log.h>
 #include <multipass/ssh/ssh_session.h>
 #include <multipass/top_catch_all.h>
 #include <multipass/utils.h>
 #include <multipass/virtual_machine_description.h>
 #include <multipass/vm_specs.h>
 #include <multipass/vm_status_monitor.h>
-#include <shared/shared_backend_utils.h>
 #include <shared/windows/powershell.h>
 #include <shared/windows/smb_mount_handler.h>
 
@@ -432,9 +431,7 @@ int mp::HyperVVirtualMachine::ssh_port()
 
 void mp::HyperVVirtualMachine::ensure_vm_is_running()
 {
-    auto is_vm_running = [this] { return state != State::off; };
-
-    mp::backend::ensure_vm_is_running_for(this, is_vm_running, "Instance shutdown during start");
+    ensure_vm_is_running_for();
 }
 
 void mp::HyperVVirtualMachine::update_state()
@@ -460,24 +457,16 @@ std::string mp::HyperVVirtualMachine::ssh_username()
     return desc.ssh_username;
 }
 
-std::string mp::HyperVVirtualMachine::management_ipv4()
+auto mp::HyperVVirtualMachine::management_ipv4() -> std::optional<IPAddress>
 {
+    // Not using cached SSH session for this because a) the underlying functions do not
+    // guarantee constness; b) we endure the penalty of creating a new session only when we
+    // don't have the IP yet.
     if (!management_ip)
-    {
-        // Not using cached SSH session for this because a) the underlying functions do not
-        // guarantee constness; b) we endure the penalty of creating a new session only when we
-        // don't have the IP yet.
-        auto result =
+        management_ip =
             remote_ip(VirtualMachine::ssh_hostname(), ssh_port(), ssh_username(), key_provider);
-        if (result)
-            management_ip.emplace(result.value());
-    }
-    return management_ip ? management_ip.value().as_string() : "UNKNOWN";
-}
 
-std::string mp::HyperVVirtualMachine::ipv6()
-{
-    return {};
+    return management_ip;
 }
 
 void mp::HyperVVirtualMachine::update_cpus(int num_cores)

src/platform/backends/hyperv/hyperv_virtual_machine.h

@@ -59,8 +59,7 @@ class HyperVVirtualMachine final : public BaseVirtualMachine
     int ssh_port() override;
     std::string ssh_hostname(std::chrono::milliseconds timeout) override;
     std::string ssh_username() override;
-    std::string management_ipv4() override;
-    std::string ipv6() override;
+    std::optional<IPAddress> management_ipv4() override;
     void ensure_vm_is_running() override;
     void update_state() override;
     void update_cpus(int num_cores) override;

src/platform/backends/qemu/linux/dnsmasq_server.cpp

@@ -109,7 +109,7 @@ std::optional<mp::IPAddress> mp::DNSMasqServer::get_ip_for(const std::string& hw
     {
         const auto fields = mp::utils::split(line, delimiter);
         if (fields.size() > 2 && fields[hw_addr_idx] == hw_addr)
-            return fields[ipv4_idx];
+            return IPAddress{fields[ipv4_idx]};
     }
     return std::nullopt;
 }