diff --git a/include/multipass/ip_address.h b/include/multipass/ip_address.h index 9ecd215f34..66d081daf4 100644 --- a/include/multipass/ip_address.h +++ b/include/multipass/ip_address.h @@ -27,13 +27,12 @@ namespace multipass { struct IPAddress { - IPAddress(std::array octets); - IPAddress(const std::string& ip_string); + explicit IPAddress(std::array 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; diff --git a/include/multipass/virtual_machine.h b/include/multipass/virtual_machine.h index 864a39d810..d3ce9a29b1 100644 --- a/include/multipass/virtual_machine.h +++ b/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 #include @@ -29,12 +27,12 @@ #include #include #include -#include #include #include 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 get_all_ipv4() = 0; - virtual std::string ipv6() = 0; + virtual std::optional management_ipv4() = 0; + virtual std::vector 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 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 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 -} diff --git a/src/client/cli/formatter/csv_formatter.cpp b/src/client/cli/formatter/csv_formatter.cpp index 4e98d6db40..2e00068dcc 100644 --- a/src/client/cli/formatter/csv_formatter.cpp +++ b/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(), ",")); diff --git a/src/client/cli/formatter/table_formatter.cpp b/src/client/cli/formatter/table_formatter.cpp index 8cef22d2d4..06e1283ce8 100644 --- a/src/client/cli/formatter/table_formatter.cpp +++ b/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:", diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index d9dc252c84..8e5503bb58 100644 --- a/src/daemon/daemon.cpp +++ b/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; diff --git a/src/daemon/runtime_instance_info_helper.cpp b/src/daemon/runtime_instance_info_helper.cpp index 76998deea5..a4f722424c 100644 --- a/src/daemon/runtime_instance_info_helper.cpp +++ b/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()); } diff --git a/src/network/ip_address.cpp b/src/network/ip_address.cpp index d352979a99..c9840b1d7e 100644 --- a/src/network/ip_address.cpp +++ b/src/network/ip_address.cpp @@ -17,7 +17,11 @@ #include +#include +#include #include +#include +#include namespace mp = multipass; @@ -63,7 +67,7 @@ mp::IPAddress::IPAddress(std::array 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)) { } diff --git a/src/platform/backends/hyperv/hyperv_virtual_machine.cpp b/src/platform/backends/hyperv/hyperv_virtual_machine.cpp index 9c6cfbc4b3..40fd3194e6 100644 --- a/src/platform/backends/hyperv/hyperv_virtual_machine.cpp +++ b/src/platform/backends/hyperv/hyperv_virtual_machine.cpp @@ -19,9 +19,9 @@ #include "hyperv_snapshot.h" #include -#include // TODO@snapshots drop #include #include +#include #include #include #include @@ -29,7 +29,6 @@ #include #include #include -#include #include #include @@ -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 { + // 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) diff --git a/src/platform/backends/hyperv/hyperv_virtual_machine.h b/src/platform/backends/hyperv/hyperv_virtual_machine.h index b05998e30c..f116caf9fe 100644 --- a/src/platform/backends/hyperv/hyperv_virtual_machine.h +++ b/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 management_ipv4() override; void ensure_vm_is_running() override; void update_state() override; void update_cpus(int num_cores) override; diff --git a/src/platform/backends/qemu/linux/dnsmasq_server.cpp b/src/platform/backends/qemu/linux/dnsmasq_server.cpp index b65659ae2f..03b11facd1 100644 --- a/src/platform/backends/qemu/linux/dnsmasq_server.cpp +++ b/src/platform/backends/qemu/linux/dnsmasq_server.cpp @@ -109,7 +109,7 @@ std::optional 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; } diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index 6263fc34a1..b23087bbe9 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -22,14 +22,13 @@ #include "qemu_vm_process_spec.h" #include "qemu_vmstate_process_spec.h" -#include - +#include #include #include +#include #include #include #include -#include #include #include #include @@ -46,7 +45,8 @@ #include namespace mp = multipass; -namespace mpl = multipass::logging; +namespace mpl = mp::logging; +namespace mpu = mp::utils; using namespace std::chrono_literals; @@ -462,17 +462,12 @@ void mp::QemuVirtualMachine::on_error() void mp::QemuVirtualMachine::on_shutdown() { { - std::unique_lock lock{state_mutex}; + std::unique_lock lock{state_mutex}; auto current_state = state; state = State::off; if (current_state == State::starting) - { - if (!saved_error_msg.empty() && saved_error_msg.back() != '\n') - saved_error_msg.append("\n"); - saved_error_msg.append(fmt::format("{}: shutdown called while starting", vm_name)); state_wait.wait(lock, [this] { return shutdown_while_starting; }); - } management_ip = std::nullopt; drop_ssh_session(); @@ -518,18 +513,15 @@ void mp::QemuVirtualMachine::ensure_vm_is_running() } } - auto is_vm_running = [this] { return (vm_process && vm_process->running()); }; - - mp::backend::ensure_vm_is_running_for(this, is_vm_running, saved_error_msg); + ensure_vm_is_running_for(saved_error_msg); } std::string mp::QemuVirtualMachine::ssh_hostname(std::chrono::milliseconds timeout) { - auto get_ip = [this]() -> std::optional { - return qemu_platform->get_ip_for(desc.default_mac_address); - }; + fetch_ip(timeout); - return mp::backend::ip_address_for(this, get_ip, timeout); + assert(management_ip && "Should have thrown otherwise"); + return management_ip->as_string(); } std::string mp::QemuVirtualMachine::ssh_username() @@ -537,23 +529,12 @@ std::string mp::QemuVirtualMachine::ssh_username() return desc.ssh_username; } -std::string mp::QemuVirtualMachine::management_ipv4() +auto mp::QemuVirtualMachine::management_ipv4() -> std::optional { if (!management_ip) - { - auto result = qemu_platform->get_ip_for(desc.default_mac_address); - if (result) - management_ip.emplace(result.value()); - else - return "UNKNOWN"; - } - - return management_ip.value().as_string(); -} + management_ip = qemu_platform->get_ip_for(desc.default_mac_address); -std::string mp::QemuVirtualMachine::ipv6() -{ - return {}; + return management_ip; } void mp::QemuVirtualMachine::wait_until_ssh_up(std::chrono::milliseconds timeout) @@ -820,9 +801,33 @@ auto mp::QemuVirtualMachine::make_specific_snapshot(const std::string& snapshot_ *this, desc); } +bool multipass::QemuVirtualMachine::unplugged() const +{ + return BaseVirtualMachine::unplugged() || !vm_process || !vm_process->running(); +} auto mp::QemuVirtualMachine::make_specific_snapshot(const QString& filename) -> std::shared_ptr { return std::make_shared(filename, *this, desc); } + +void mp::QemuVirtualMachine::fetch_ip(std::chrono::milliseconds timeout) +{ + if (!management_ip) + { + auto action = [this] { + ensure_vm_is_running(); + return ((management_ip = qemu_platform->get_ip_for(desc.default_mac_address))) + ? mpu::TimeoutAction::done + : mpu::TimeoutAction::retry; + }; + + auto on_timeout = [this, &timeout] { + state = State::unknown; + throw InternalTimeoutException{"determine IP address", timeout}; + }; + + mpu::try_action_for(on_timeout, timeout, action); + } +} diff --git a/src/platform/backends/qemu/qemu_virtual_machine.h b/src/platform/backends/qemu/qemu_virtual_machine.h index 5b3bb335ad..2e91bef47b 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.h +++ b/src/platform/backends/qemu/qemu_virtual_machine.h @@ -28,6 +28,7 @@ #include #include +#include #include namespace multipass @@ -56,8 +57,7 @@ class QemuVirtualMachine : public QObject, 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 management_ipv4() override; void ensure_vm_is_running() override; void wait_until_ssh_up(std::chrono::milliseconds timeout) override; void update_state() override; @@ -91,6 +91,8 @@ class QemuVirtualMachine : public QObject, public BaseVirtualMachine const VMSpecs& specs, std::shared_ptr parent) override; + bool unplugged() const override; + private: void on_started(); void on_error(); @@ -101,6 +103,8 @@ class QemuVirtualMachine : public QObject, public BaseVirtualMachine void connect_vm_signals(); void disconnect_vm_signals(); + void fetch_ip(std::chrono::milliseconds timeout); + void remove_snapshots_from_backend() const; VirtualMachineDescription desc; diff --git a/src/platform/backends/shared/base_virtual_machine.cpp b/src/platform/backends/shared/base_virtual_machine.cpp index 547ac76518..c83c63cccb 100644 --- a/src/platform/backends/shared/base_virtual_machine.cpp +++ b/src/platform/backends/shared/base_virtual_machine.cpp @@ -36,11 +36,13 @@ #include #include +#include +#include #include -#include #include #include +#include namespace mp = multipass; namespace mpl = multipass::logging; @@ -138,18 +140,18 @@ std::optional wait_until_ssh_up_helper(mp::VirtualMachine* virtu } } // namespace -mp::BaseVirtualMachine::BaseVirtualMachine(VirtualMachine::State state, - const std::string& vm_name, +mp::BaseVirtualMachine::BaseVirtualMachine(const std::string& vm_name, const SSHKeyProvider& key_provider, const Path& instance_dir) - : VirtualMachine{state, vm_name, instance_dir}, key_provider{key_provider} + : VirtualMachine{vm_name}, key_provider{key_provider}, instance_dir{instance_dir} { } -mp::BaseVirtualMachine::BaseVirtualMachine(const std::string& vm_name, +mp::BaseVirtualMachine::BaseVirtualMachine(State state, + const std::string& vm_name, const SSHKeyProvider& key_provider, const Path& instance_dir) - : VirtualMachine{vm_name, instance_dir}, key_provider{key_provider} + : VirtualMachine{state, vm_name}, key_provider{key_provider}, instance_dir{instance_dir} { } @@ -280,6 +282,27 @@ void mp::BaseVirtualMachine::renew_ssh_session() ssh_session.emplace(ssh_hostname(), ssh_port(), ssh_username(), key_provider); } +bool multipass::BaseVirtualMachine::unplugged() const +{ + return state == State::off || state == State::stopped; +} + +void mp::BaseVirtualMachine::ensure_vm_is_running_for(const std::string& detail) +{ + const std::lock_guard lock{state_mutex}; + if (unplugged()) + { + shutdown_while_starting = true; + state_wait.notify_all(); + + using namespace std::literals; // TODO@no-merge make this central? + auto msg = "Instance shutdown during start"s; + if (!detail.empty()) + msg += fmt::format(": {}", detail); + + throw StartException(vm_name, msg); + } +} void mp::BaseVirtualMachine::wait_until_ssh_up(std::chrono::milliseconds timeout) { @@ -314,9 +337,9 @@ void mp::BaseVirtualMachine::wait_for_cloud_init(std::chrono::milliseconds timeo mp::utils::try_action_for(on_timeout, timeout, action); } -std::vector mp::BaseVirtualMachine::get_all_ipv4() +auto mp::BaseVirtualMachine::get_all_ipv4() -> std::vector { - std::vector all_ipv4; + std::vector all_ipv4; if (MP_UTILS.is_running(current_state())) { @@ -333,9 +356,9 @@ std::vector mp::BaseVirtualMachine::get_all_ipv4() while (ip_it.hasNext()) { auto ip_match = ip_it.next(); - auto ip = ip_match.captured(1).toStdString(); + auto ip_str = ip_match.captured(1).toStdString(); - all_ipv4.push_back(ip); + all_ipv4.push_back(IPAddress{ip_str}); } } catch (const SSHException& e) diff --git a/src/platform/backends/shared/base_virtual_machine.h b/src/platform/backends/shared/base_virtual_machine.h index f0201192be..196293d79c 100644 --- a/src/platform/backends/shared/base_virtual_machine.h +++ b/src/platform/backends/shared/base_virtual_machine.h @@ -18,16 +18,12 @@ #pragma once #include -#include +#include +#include #include #include #include -#include - -#include -#include - #include #include #include @@ -54,7 +50,7 @@ class BaseVirtualMachine : public VirtualMachine void wait_until_ssh_up(std::chrono::milliseconds timeout) override; void wait_for_cloud_init(std::chrono::milliseconds timeout) override; - std::vector get_all_ipv4() override; + std::vector get_all_ipv4() override; void add_network_interface(int index, const std::string& default_mac_addr, const NetworkInterface& extra_interface) override @@ -87,6 +83,8 @@ class BaseVirtualMachine : public VirtualMachine std::vector get_childrens_names(const Snapshot* parent) const override; int get_snapshot_count() const override; + QDir instance_directory() const override; + protected: virtual std::shared_ptr make_specific_snapshot(const QString& filename); virtual std::shared_ptr make_specific_snapshot(const std::string& snapshot_name, @@ -97,6 +95,9 @@ class BaseVirtualMachine : public VirtualMachine virtual void drop_ssh_session(); // virtual to allow mocking void renew_ssh_session(); + virtual bool unplugged() const; + void ensure_vm_is_running_for(const std::string& detail = ""); + virtual void add_extra_interface_to_instance_cloud_init( const std::string& default_mac_addr, const NetworkInterface& extra_interface) const; @@ -156,8 +157,13 @@ class BaseVirtualMachine : public VirtualMachine void delete_snapshot_helper(std::shared_ptr& snapshot); +public: + bool shutdown_while_starting{false}; // TODO@no-merge hide + protected: const SSHKeyProvider& key_provider; + const QDir instance_dir; + std::optional management_ip; private: std::optional ssh_session = std::nullopt; @@ -179,3 +185,8 @@ inline int multipass::BaseVirtualMachine::get_snapshot_count() const const std::unique_lock lock{snapshot_mutex}; return snapshot_count; } + +inline QDir multipass::BaseVirtualMachine::instance_directory() const +{ + return instance_dir; +} diff --git a/src/platform/backends/shared/shared_backend_utils.h b/src/platform/backends/shared/shared_backend_utils.h deleted file mode 100644 index 7ce4d7cec1..0000000000 --- a/src/platform/backends/shared/shared_backend_utils.h +++ /dev/null @@ -1,80 +0,0 @@ -/* - * Copyright (C) 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 - * the Free Software Foundation; version 3. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - * - */ - -#pragma once - -#include -#include -#include -#include - -#include -#include - -namespace multipass -{ -namespace backend -{ -using namespace std::chrono_literals; - -template -std::string ip_address_for(VirtualMachine* virtual_machine, - Callable&& get_ip, - std::chrono::milliseconds timeout) -{ - if (!virtual_machine->management_ip) - { - auto action = [virtual_machine, get_ip] { - virtual_machine->ensure_vm_is_running(); - auto result = get_ip(); - if (result) - { - virtual_machine->management_ip.emplace(*result); - return utils::TimeoutAction::done; - } - else - { - return utils::TimeoutAction::retry; - } - }; - - auto on_timeout = [virtual_machine, &timeout] { - virtual_machine->state = VirtualMachine::State::unknown; - throw InternalTimeoutException{"determine IP address", timeout}; - }; - - utils::try_action_for(on_timeout, timeout, action); - } - - return virtual_machine->management_ip->as_string(); -} - -template -void ensure_vm_is_running_for(VirtualMachine* virtual_machine, - Callable&& is_vm_running, - const std::string& msg) -{ - std::lock_guardstate_mutex)> lock{virtual_machine->state_mutex}; - if (!is_vm_running()) - { - virtual_machine->shutdown_while_starting = true; - virtual_machine->state_wait.notify_all(); - throw StartException(virtual_machine->vm_name, msg); - } -} -} // namespace backend -} // namespace multipass diff --git a/src/platform/backends/virtualbox/virtualbox_virtual_machine.cpp b/src/platform/backends/virtualbox/virtualbox_virtual_machine.cpp index f950577b87..6264814d84 100644 --- a/src/platform/backends/virtualbox/virtualbox_virtual_machine.cpp +++ b/src/platform/backends/virtualbox/virtualbox_virtual_machine.cpp @@ -18,8 +18,8 @@ #include "virtualbox_virtual_machine.h" #include "virtualbox_snapshot.h" -#include #include +#include #include #include #include @@ -30,8 +30,6 @@ #include #include -#include - #include #include @@ -493,9 +491,7 @@ int mp::VirtualBoxVirtualMachine::ssh_port() void mp::VirtualBoxVirtualMachine::ensure_vm_is_running() { - auto is_vm_running = [this] { return state != State::stopped; }; - - mp::backend::ensure_vm_is_running_for(this, is_vm_running, "Instance shutdown during start"); + ensure_vm_is_running_for(); } void mp::VirtualBoxVirtualMachine::update_state() @@ -513,26 +509,22 @@ std::string mp::VirtualBoxVirtualMachine::ssh_username() return desc.ssh_username; } -std::string mp::VirtualBoxVirtualMachine::management_ipv4() +auto mp::VirtualBoxVirtualMachine::management_ipv4() -> std::optional { - return "N/A"; + return std::nullopt; } -std::vector mp::VirtualBoxVirtualMachine::get_all_ipv4() +auto mp::VirtualBoxVirtualMachine::get_all_ipv4() -> std::vector { using namespace std; + const auto internal_ip = IPAddress{"10.0.2.15"}; auto all_ipv4 = BaseVirtualMachine::get_all_ipv4(); - all_ipv4.erase(remove(begin(all_ipv4), end(all_ipv4), "10.0.2.15"), end(all_ipv4)); + std::erase(all_ipv4, internal_ip); return all_ipv4; } -std::string mp::VirtualBoxVirtualMachine::ipv6() -{ - return {}; -} - void mp::VirtualBoxVirtualMachine::update_cpus(int num_cores) { assert(num_cores > 0); diff --git a/src/platform/backends/virtualbox/virtualbox_virtual_machine.h b/src/platform/backends/virtualbox/virtualbox_virtual_machine.h index 523e06ed69..ae893751ab 100644 --- a/src/platform/backends/virtualbox/virtualbox_virtual_machine.h +++ b/src/platform/backends/virtualbox/virtualbox_virtual_machine.h @@ -54,9 +54,8 @@ class VirtualBoxVirtualMachine 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::vector get_all_ipv4() override; - std::string ipv6() override; + std::optional management_ipv4() override; + std::vector get_all_ipv4() override; void ensure_vm_is_running() override; void update_state() override; void update_cpus(int num_cores) override; diff --git a/src/rpc/multipass.proto b/src/rpc/multipass.proto index 3b12b4c5b7..e12edee5a4 100644 --- a/src/rpc/multipass.proto +++ b/src/rpc/multipass.proto @@ -211,7 +211,6 @@ message InstanceDetails { string memory_usage = 5; string disk_usage = 6; repeated string ipv4 = 7; - repeated string ipv6 = 8; optional int32 num_snapshots = 9; string cpu_times = 10; string uptime = 11; @@ -262,7 +261,6 @@ message ListVMInstance { string name = 1; InstanceStatus instance_status = 2; repeated string ipv4 = 3; - repeated string ipv6 = 4; string current_release = 5; } diff --git a/tests/linux/test_backend_utils.cpp b/tests/linux/test_backend_utils.cpp index f0c98ca975..9614ca5460 100644 --- a/tests/linux/test_backend_utils.cpp +++ b/tests/linux/test_backend_utils.cpp @@ -23,7 +23,6 @@ #include "tests/mock_singleton_helpers.h" #include "tests/mock_utils.h" -#include #include #include diff --git a/tests/mock_virtual_machine.h b/tests/mock_virtual_machine.h index 30174bae37..19c43e0f4c 100644 --- a/tests/mock_virtual_machine.h +++ b/tests/mock_virtual_machine.h @@ -20,42 +20,54 @@ #include "common.h" #include "temp_dir.h" +#include #include #include #include #include +#include using namespace testing; -namespace multipass -{ -namespace test +namespace multipass::test { template >> struct MockVirtualMachineT : public T { template - MockVirtualMachineT(Args&&... args) + explicit MockVirtualMachineT(Args&&... args) : MockVirtualMachineT{std::make_unique(), std::forward(args)...} { } template - MockVirtualMachineT(std::unique_ptr&& tmp_dir, Args&&... args) + requires(std::is_same_v) + explicit MockVirtualMachineT(std::unique_ptr&& tmp_dir, Args&&... args) + : T{std::forward(args)...}, tmp_dir{std::move(tmp_dir)} + { + setup_default_actions(); + } + + template + requires(!std::is_same_v) + explicit MockVirtualMachineT(std::unique_ptr&& tmp_dir, Args&&... args) : T{std::forward(args)..., tmp_dir->path()}, tmp_dir{std::move(tmp_dir)} { - ON_CALL(*this, current_state()) - .WillByDefault(Return(multipass::VirtualMachine::State::off)); - ON_CALL(*this, ssh_port()).WillByDefault(Return(42)); + setup_default_actions(); + } + + void setup_default_actions() + { + ON_CALL(*this, current_state).WillByDefault(Return(multipass::VirtualMachine::State::off)); + ON_CALL(*this, ssh_port).WillByDefault(Return(42)); ON_CALL(*this, ssh_hostname()).WillByDefault(Return("localhost")); ON_CALL(*this, ssh_hostname(_)).WillByDefault(Return("localhost")); - ON_CALL(*this, ssh_username()).WillByDefault(Return("ubuntu")); - ON_CALL(*this, management_ipv4()).WillByDefault(Return("0.0.0.0")); - ON_CALL(*this, get_all_ipv4()) - .WillByDefault(Return(std::vector{"192.168.2.123"})); - ON_CALL(*this, ipv6()).WillByDefault(Return("::/0")); + ON_CALL(*this, ssh_username).WillByDefault(Return("ubuntu")); + ON_CALL(*this, management_ipv4).WillByDefault(Return(IPAddress{"0.0.0.0"})); + ON_CALL(*this, get_all_ipv4).WillByDefault(Return(std::vector{IPAddress{"192.168.2.123"}})); + ON_CALL(*this, instance_directory).WillByDefault(Return(this->tmp_dir->path())); } MOCK_METHOD(void, start, (), (override)); @@ -66,10 +78,8 @@ struct MockVirtualMachineT : public T MOCK_METHOD(std::string, ssh_hostname, (), (override)); MOCK_METHOD(std::string, ssh_hostname, (std::chrono::milliseconds), (override)); MOCK_METHOD(std::string, ssh_username, (), (override)); - MOCK_METHOD(std::string, management_ipv4, (), (override)); - MOCK_METHOD(std::vector, get_all_ipv4, (), (override)); - MOCK_METHOD(std::string, ipv6, (), (override)); - + MOCK_METHOD(std::optional, management_ipv4, (), (override)); + MOCK_METHOD(std::vector, get_all_ipv4, (), (override)); MOCK_METHOD(std::string, ssh_exec, (const std::string& cmd, bool whisper), (override)); std::string ssh_exec(const std::string& cmd) { @@ -116,10 +126,10 @@ struct MockVirtualMachineT : public T (const Snapshot*), (const, override)); MOCK_METHOD(int, get_snapshot_count, (), (const, override)); + MOCK_METHOD(QDir, instance_directory, (), (const, override)); std::unique_ptr tmp_dir; }; using MockVirtualMachine = MockVirtualMachineT<>; -} // namespace test -} // namespace multipass +} // namespace multipass::test diff --git a/tests/qemu/test_qemu_backend.cpp b/tests/qemu/test_qemu_backend.cpp index 92d2cd0ecf..42024b31f2 100644 --- a/tests/qemu/test_qemu_backend.cpp +++ b/tests/qemu/test_qemu_backend.cpp @@ -947,7 +947,7 @@ TEST_F(QemuBackend, sshHostnameReturnsExpectedValue) TEST_F(QemuBackend, getsManagementIp) { - const std::string expected_ip{"10.10.0.35"}; + const mp::IPAddress expected_ip{"10.10.0.35"}; NiceMock mock_qemu_platform; EXPECT_CALL(mock_qemu_platform, get_ip_for(_)).WillOnce(Return(expected_ip)); @@ -977,7 +977,7 @@ TEST_F(QemuBackend, failsToGetManagementIpIfDnsmasqDoesNotReturnAnIp) machine.start(); machine.state = mp::VirtualMachine::State::running; - EXPECT_EQ(machine.management_ipv4(), "UNKNOWN"); + EXPECT_EQ(machine.management_ipv4(), std::nullopt); } TEST_F(QemuBackend, sshHostnameTimeoutThrowsAndSetsUnknownState) diff --git a/tests/stub_virtual_machine.h b/tests/stub_virtual_machine.h index 1931ca438b..b46d303de0 100644 --- a/tests/stub_virtual_machine.h +++ b/tests/stub_virtual_machine.h @@ -21,6 +21,7 @@ #include "stub_snapshot.h" #include "temp_dir.h" +#include #include namespace multipass @@ -33,13 +34,13 @@ struct StubVirtualMachine final : public multipass::VirtualMachine { } - StubVirtualMachine(const std::string& name) + explicit StubVirtualMachine(const std::string& name) : StubVirtualMachine{name, std::make_unique()} { } StubVirtualMachine(const std::string& name, std::unique_ptr tmp_dir) - : VirtualMachine{name, tmp_dir->path()}, tmp_dir{std::move(tmp_dir)} + : VirtualMachine{name}, tmp_dir{std::move(tmp_dir)} { } @@ -75,19 +76,14 @@ struct StubVirtualMachine final : public multipass::VirtualMachine return "ubuntu"; } - std::string management_ipv4() override + std::optional management_ipv4() override { - return {}; - } - - std::vector get_all_ipv4() override - { - return std::vector{"192.168.2.123"}; + return std::nullopt; } - std::string ipv6() override + std::vector get_all_ipv4() override { - return {}; + return {IPAddress{"192.168.2.123"}}; } std::string ssh_exec(const std::string& cmd, bool whisper = false) override @@ -197,6 +193,11 @@ struct StubVirtualMachine final : public multipass::VirtualMachine return 0; } + QDir instance_directory() const override + { + return tmp_dir->path(); + } + StubSnapshot snapshot; std::unique_ptr tmp_dir; }; diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 3653758c06..1902ef0cc8 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -168,14 +169,9 @@ struct StubBaseVirtualMachine : public mp::BaseVirtualMachine return "ubuntu"; } - std::string management_ipv4() override + std::optional management_ipv4() override { - return "1.2.3.4"; - } - - std::string ipv6() override - { - return ""; + return mp::IPAddress{"1.2.3.4"}; } void wait_until_ssh_up(std::chrono::milliseconds) override @@ -314,7 +310,7 @@ struct IpTestParams { int exit_status; std::string output; - std::vector expected_ips; + std::vector expected_ips; }; struct IpExecution : public BaseVM, public WithParamInterface @@ -363,17 +359,20 @@ TEST_P(IpExecution, getAllIpv4WorksWhenSshWorks) INSTANTIATE_TEST_SUITE_P( BaseVM, IpExecution, - Values( - IpTestParams{0, "eth0 UP 192.168.2.168/24 \n", {"192.168.2.168"}}, - IpTestParams{0, - "eth1 UP 192.168.2.169/24 metric 100 \n", - {"192.168.2.169"}}, - IpTestParams{0, - "wlp4s0 UP 192.168.2.8/24 \n" - "virbr0 DOWN 192.168.3.1/24 \n" - "tun0 UNKNOWN 10.172.66.5/18 \n", - {"192.168.2.8", "192.168.3.1", "10.172.66.5"}}, - IpTestParams{0, "", {}})); + Values(IpTestParams{0, + "eth0 UP 192.168.2.168/24 \n", + {mp::IPAddress{"192.168.2.168"}}}, + IpTestParams{0, + "eth1 UP 192.168.2.169/24 metric 100 \n", + {mp::IPAddress{"192.168.2.169"}}}, + IpTestParams{0, + "wlp4s0 UP 192.168.2.8/24 \n" + "virbr0 DOWN 192.168.3.1/24 \n" + "tun0 UNKNOWN 10.172.66.5/18 \n", + {mp::IPAddress{"192.168.2.8"}, + mp::IPAddress{"192.168.3.1"}, + mp::IPAddress{"10.172.66.5"}}}, + IpTestParams{0, "", {}})); TEST_F(BaseVM, startsWithNoSnapshots) { diff --git a/tests/test_data/formatters/csv/empty_list_reply.csv b/tests/test_data/formatters/csv/empty_list_reply.csv index 19dfdbb023..f3261f63df 100644 --- a/tests/test_data/formatters/csv/empty_list_reply.csv +++ b/tests/test_data/formatters/csv/empty_list_reply.csv @@ -1 +1 @@ -Name,State,IPv4,IPv6,Release,AllIPv4 +Name,State,IPv4,Release,AllIPv4 diff --git a/tests/test_data/formatters/csv/multiple_instances_info_reply.csv b/tests/test_data/formatters/csv/multiple_instances_info_reply.csv index d2c3001ee7..3ccff780c0 100644 --- a/tests/test_data/formatters/csv/multiple_instances_info_reply.csv +++ b/tests/test_data/formatters/csv/multiple_instances_info_reply.csv @@ -1,3 +1,3 @@ -Name,State,Ipv4,Ipv6,Release,Image hash,Image release,Load,Disk usage,Disk total,Memory usage,Memory total,Mounts,AllIPv4,CPU(s),Snapshots -bogus-instance,Running,10.21.124.56,,Ubuntu 16.04.3 LTS,1797c5c82016c1e65f4008fcf89deae3a044ef76087a9ec5b907c6d64a3609ac,Ubuntu 16.04 LTS,0.03 0.10 0.15,1932735284,6764573492,38797312,1610612736,/home/user/source => source,10.21.124.56,4,1 -bombastic,Stopped,,,,ab5191cc172564e7cc0eafd397312a32598823e645279c820f0935393aead509,Ubuntu 18.04 LTS,,,,,,,,,3 +Name,State,Ipv4,Release,Image hash,Image release,Load,Disk usage,Disk total,Memory usage,Memory total,Mounts,AllIPv4,CPU(s),Snapshots +bogus-instance,Running,10.21.124.56,Ubuntu 16.04.3 LTS,1797c5c82016c1e65f4008fcf89deae3a044ef76087a9ec5b907c6d64a3609ac,Ubuntu 16.04 LTS,0.03 0.10 0.15,1932735284,6764573492,38797312,1610612736,/home/user/source => source,10.21.124.56,4,1 +bombastic,Stopped,,,ab5191cc172564e7cc0eafd397312a32598823e645279c820f0935393aead509,Ubuntu 18.04 LTS,,,,,,,,,3 diff --git a/tests/test_data/formatters/csv/multiple_instances_list_reply.csv b/tests/test_data/formatters/csv/multiple_instances_list_reply.csv index c5d481156b..10af78d0ce 100644 --- a/tests/test_data/formatters/csv/multiple_instances_list_reply.csv +++ b/tests/test_data/formatters/csv/multiple_instances_list_reply.csv @@ -1,3 +1,3 @@ -Name,State,IPv4,IPv6,Release,AllIPv4 -bogus-instance,Running,10.21.124.56,,Ubuntu 16.04 LTS,"10.21.124.56" -bombastic,Stopped,,,Ubuntu 18.04 LTS,"" +Name,State,IPv4,Release,AllIPv4 +bogus-instance,Running,10.21.124.56,Ubuntu 16.04 LTS,"10.21.124.56" +bombastic,Stopped,,Ubuntu 18.04 LTS,"" diff --git a/tests/test_data/formatters/csv/single_instance_info_reply.csv b/tests/test_data/formatters/csv/single_instance_info_reply.csv index 81a5645d45..5c4dc2ccc5 100644 --- a/tests/test_data/formatters/csv/single_instance_info_reply.csv +++ b/tests/test_data/formatters/csv/single_instance_info_reply.csv @@ -1,2 +1,2 @@ -Name,State,Ipv4,Ipv6,Release,Image hash,Image release,Load,Disk usage,Disk total,Memory usage,Memory total,Mounts,AllIPv4,CPU(s),Snapshots -foo,Running,10.168.32.2,2001:67c:1562:8007::aac:423a,Ubuntu 16.04.3 LTS,1797c5c82016c1e65f4008fcf89deae3a044ef76087a9ec5b907c6d64a3609ac,Ubuntu 16.04 LTS,0.45 0.51 0.15,1288490188,5153960756,60817408,1503238554,/home/user/foo => foo;/home/user/test_dir => test_dir,10.168.32.2;200.3.123.29,1,0 +Name,State,Ipv4,Release,Image hash,Image release,Load,Disk usage,Disk total,Memory usage,Memory total,Mounts,AllIPv4,CPU(s),Snapshots +foo,Running,10.168.32.2,Ubuntu 16.04.3 LTS,1797c5c82016c1e65f4008fcf89deae3a044ef76087a9ec5b907c6d64a3609ac,Ubuntu 16.04 LTS,0.45 0.51 0.15,1288490188,5153960756,60817408,1503238554,/home/user/foo => foo;/home/user/test_dir => test_dir,10.168.32.2;200.3.123.29,1,0 diff --git a/tests/test_data/formatters/csv/single_instance_list_reply.csv b/tests/test_data/formatters/csv/single_instance_list_reply.csv index a733af8291..79d621353b 100644 --- a/tests/test_data/formatters/csv/single_instance_list_reply.csv +++ b/tests/test_data/formatters/csv/single_instance_list_reply.csv @@ -1,2 +1,2 @@ -Name,State,IPv4,IPv6,Release,AllIPv4 -foo,Running,10.168.32.2,fdde:2681:7a2::4ca,Ubuntu 16.04 LTS,"10.168.32.2,200.3.123.30" +Name,State,IPv4,Release,AllIPv4 +foo,Running,10.168.32.2,Ubuntu 16.04 LTS,"10.168.32.2,200.3.123.30" diff --git a/tests/test_data/formatters/csv/unsorted_list_reply.csv b/tests/test_data/formatters/csv/unsorted_list_reply.csv index 188bde9880..9404eb2910 100644 --- a/tests/test_data/formatters/csv/unsorted_list_reply.csv +++ b/tests/test_data/formatters/csv/unsorted_list_reply.csv @@ -1,5 +1,5 @@ -Name,State,IPv4,IPv6,Release,AllIPv4 -trusty-190611-1529,Deleted,,,Not Available,"" -trusty-190611-1535,Stopped,,,Fedora 42,"" -trusty-190611-1539,Suspended,,,Not Available,"" -trusty-190611-1542,Running,,,Debian 12,"" +Name,State,IPv4,Release,AllIPv4 +trusty-190611-1529,Deleted,,Not Available,"" +trusty-190611-1535,Stopped,,Fedora 42,"" +trusty-190611-1539,Suspended,,Not Available,"" +trusty-190611-1542,Running,,Debian 12,"" diff --git a/tests/test_data/formatters/table/single_instance_info_reply.txt b/tests/test_data/formatters/table/single_instance_info_reply.txt index 0bbf3487b6..4c2dd01418 100644 --- a/tests/test_data/formatters/table/single_instance_info_reply.txt +++ b/tests/test_data/formatters/table/single_instance_info_reply.txt @@ -3,8 +3,6 @@ State: Running Snapshots: 0 IPv4: 10.168.32.2 200.3.123.29 -IPv6: 2001:67c:1562:8007::aac:423a - fd52:2ccf:f758:0:a342:79b5:e2ba:e05e Release: Ubuntu 16.04.3 LTS Image hash: 1797c5c82016 (Ubuntu 16.04 LTS) CPU(s): 1 diff --git a/tests/test_output_formatter.cpp b/tests/test_output_formatter.cpp index 361b176e3a..bbb50e628c 100644 --- a/tests/test_output_formatter.cpp +++ b/tests/test_output_formatter.cpp @@ -66,8 +66,6 @@ auto construct_single_instance_list_reply() list_entry->set_current_release("Ubuntu 16.04 LTS"); list_entry->add_ipv4("10.168.32.2"); list_entry->add_ipv4("200.3.123.30"); - list_entry->add_ipv6("fdde:2681:7a2::4ca"); - list_entry->add_ipv6("fe80::1c3c:b703:d561:a00"); return list_reply; } @@ -300,8 +298,6 @@ auto construct_single_instance_info_reply() info_entry->mutable_instance_info()->set_current_release("Ubuntu 16.04.3 LTS"); info_entry->mutable_instance_info()->add_ipv4("10.168.32.2"); info_entry->mutable_instance_info()->add_ipv4("200.3.123.29"); - info_entry->mutable_instance_info()->add_ipv6("2001:67c:1562:8007::aac:423a"); - info_entry->mutable_instance_info()->add_ipv6("fd52:2ccf:f758:0:a342:79b5:e2ba:e05e"); info_entry->mutable_instance_info()->set_num_snapshots(0); return info_reply;