From fb5142a5a02e254be22d58606a1b39bce1c5f4c5 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 16:01:04 +0100 Subject: [PATCH 01/20] [todos] Remove or update outdated snapshots TODOs --- include/multipass/snapshot.h | 2 +- src/client/cli/formatter/table_formatter.cpp | 3 ++- src/platform/backends/libvirt/libvirt_virtual_machine.cpp | 4 ++-- src/platform/backends/lxd/lxd_virtual_machine.cpp | 4 ++-- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/include/multipass/snapshot.h b/include/multipass/snapshot.h index b0abb949c6..10baafe4c2 100644 --- a/include/multipass/snapshot.h +++ b/include/multipass/snapshot.h @@ -35,7 +35,7 @@ struct VMMount; class Snapshot : private DisabledCopyMove { -public: // TODO@snapshots drop any accessors that we end up not needing +public: virtual ~Snapshot() = default; virtual int get_index() const = 0; diff --git a/src/client/cli/formatter/table_formatter.cpp b/src/client/cli/formatter/table_formatter.cpp index 9756d9f5f3..06656fcbf7 100644 --- a/src/client/cli/formatter/table_formatter.cpp +++ b/src/client/cli/formatter/table_formatter.cpp @@ -103,7 +103,8 @@ std::string generate_snapshot_details(const mp::DetailedInfoItem& item) fmt::format_to(std::back_inserter(buf), "{}\n", *child); } - // TODO@snapshots split and align string if it extends onto several lines + /* TODO split and align string if it extends onto several lines; but actually better implement generic word-wrapping + for all output, taking both terminal width and current indentation level into account */ fmt::format_to(std::back_inserter(buf), "{:<16}{}\n", "Comment:", diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp index f811d7d3c4..afe9b3cc6e 100644 --- a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp +++ b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp @@ -552,10 +552,10 @@ auto mp::LibVirtVirtualMachine::make_specific_snapshot(const std::string& /*snap std::shared_ptr /*parent*/) -> std::shared_ptr { - throw NotImplementedOnThisBackendException{"Snapshots"}; // TODO@snapshots + throw NotImplementedOnThisBackendException{"Snapshots"}; } auto mp::LibVirtVirtualMachine::make_specific_snapshot(const QString& /*filename*/) -> std::shared_ptr { - throw NotImplementedOnThisBackendException{"Snapshots"}; // TODO@snapshots + throw NotImplementedOnThisBackendException{"Snapshots"}; } diff --git a/src/platform/backends/lxd/lxd_virtual_machine.cpp b/src/platform/backends/lxd/lxd_virtual_machine.cpp index 5017b0e479..09a340347b 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.cpp +++ b/src/platform/backends/lxd/lxd_virtual_machine.cpp @@ -492,10 +492,10 @@ auto mp::LXDVirtualMachine::make_specific_snapshot(const std::string& snapshot_n const VMSpecs& specs, std::shared_ptr parent) -> std::shared_ptr { - throw NotImplementedOnThisBackendException{"Snapshots"}; // TODO@snapshots + throw NotImplementedOnThisBackendException{"Snapshots"}; } std::shared_ptr mp::LXDVirtualMachine::make_specific_snapshot(const QString& /*filename*/) { - throw NotImplementedOnThisBackendException{"Snapshots"}; // TODO@snapshots + throw NotImplementedOnThisBackendException{"Snapshots"}; } From fb988ab9269fbbdf0d473426c948a6876c7020f6 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 12:52:22 +0100 Subject: [PATCH 02/20] [snapshot] Mark a couple of accessors as noexcept --- include/multipass/snapshot.h | 4 ++-- src/platform/backends/shared/base_snapshot.h | 9 ++++----- tests/stub_snapshot.h | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/include/multipass/snapshot.h b/include/multipass/snapshot.h index 10baafe4c2..dc0f10eee0 100644 --- a/include/multipass/snapshot.h +++ b/include/multipass/snapshot.h @@ -38,10 +38,10 @@ class Snapshot : private DisabledCopyMove public: virtual ~Snapshot() = default; - virtual int get_index() const = 0; + virtual int get_index() const noexcept = 0; virtual std::string get_name() const = 0; virtual std::string get_comment() const = 0; - virtual QDateTime get_creation_timestamp() const = 0; + virtual QDateTime get_creation_timestamp() const noexcept = 0; virtual int get_num_cores() const noexcept = 0; virtual MemorySize get_mem_size() const noexcept = 0; virtual MemorySize get_disk_space() const noexcept = 0; diff --git a/src/platform/backends/shared/base_snapshot.h b/src/platform/backends/shared/base_snapshot.h index f67c925068..a07afcb0c9 100644 --- a/src/platform/backends/shared/base_snapshot.h +++ b/src/platform/backends/shared/base_snapshot.h @@ -42,11 +42,10 @@ class BaseSnapshot : public Snapshot const VirtualMachine& vm); BaseSnapshot(const QString& filename, VirtualMachine& vm); - // TODO@snapshots tag as noexcept those that can be - int get_index() const override; + int get_index() const noexcept override; std::string get_name() const override; std::string get_comment() const override; - QDateTime get_creation_timestamp() const override; + QDateTime get_creation_timestamp() const noexcept override; int get_num_cores() const noexcept override; MemorySize get_mem_size() const noexcept override; MemorySize get_disk_space() const noexcept override; @@ -130,12 +129,12 @@ inline std::string multipass::BaseSnapshot::get_comment() const return comment; } -inline int multipass::BaseSnapshot::get_index() const +inline int multipass::BaseSnapshot::get_index() const noexcept { return index; } -inline QDateTime multipass::BaseSnapshot::get_creation_timestamp() const +inline QDateTime multipass::BaseSnapshot::get_creation_timestamp() const noexcept { return creation_timestamp; } diff --git a/tests/stub_snapshot.h b/tests/stub_snapshot.h index ced617d8cf..f384903ffd 100644 --- a/tests/stub_snapshot.h +++ b/tests/stub_snapshot.h @@ -57,7 +57,7 @@ struct StubSnapshot : public Snapshot return nullptr; } - int get_index() const override + int get_index() const noexcept override { return 0; } From 96763754ef1635eda4c8ad4bc493d7bd6ef09a85 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 12:55:43 +0100 Subject: [PATCH 03/20] [vm] Remove noexcept from `view_snapshots` --- include/multipass/virtual_machine.h | 2 +- src/platform/backends/shared/base_virtual_machine.cpp | 2 +- src/platform/backends/shared/base_virtual_machine.h | 2 +- tests/stub_virtual_machine.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/multipass/virtual_machine.h b/include/multipass/virtual_machine.h index 8dc9cde7e6..1a3716f6a0 100644 --- a/include/multipass/virtual_machine.h +++ b/include/multipass/virtual_machine.h @@ -88,7 +88,7 @@ class VirtualMachine : private DisabledCopyMove const VMMount& mount) = 0; using SnapshotVista = std::vector>; // using vista to avoid confusion with C++ views - virtual SnapshotVista view_snapshots() const noexcept = 0; + virtual SnapshotVista view_snapshots() const = 0; virtual int get_num_snapshots() const noexcept = 0; virtual std::shared_ptr get_snapshot(const std::string& name) const = 0; diff --git a/src/platform/backends/shared/base_virtual_machine.cpp b/src/platform/backends/shared/base_virtual_machine.cpp index 6717ef045e..f9b97e54ad 100644 --- a/src/platform/backends/shared/base_virtual_machine.cpp +++ b/src/platform/backends/shared/base_virtual_machine.cpp @@ -121,7 +121,7 @@ std::vector BaseVirtualMachine::get_all_ipv4(const SSHKeyProvider& return all_ipv4; } -auto BaseVirtualMachine::view_snapshots() const noexcept -> SnapshotVista +auto BaseVirtualMachine::view_snapshots() const -> SnapshotVista { SnapshotVista ret; diff --git a/src/platform/backends/shared/base_virtual_machine.h b/src/platform/backends/shared/base_virtual_machine.h index 20c6dc8c16..8ab079cda8 100644 --- a/src/platform/backends/shared/base_virtual_machine.h +++ b/src/platform/backends/shared/base_virtual_machine.h @@ -50,7 +50,7 @@ class BaseVirtualMachine : public VirtualMachine throw NotImplementedOnThisBackendException("native mounts"); }; - SnapshotVista view_snapshots() const noexcept override; + SnapshotVista view_snapshots() const override; int get_num_snapshots() const noexcept override; std::shared_ptr get_snapshot(const std::string& name) const override; diff --git a/tests/stub_virtual_machine.h b/tests/stub_virtual_machine.h index c64a55ec73..f70ccabd7e 100644 --- a/tests/stub_virtual_machine.h +++ b/tests/stub_virtual_machine.h @@ -126,7 +126,7 @@ struct StubVirtualMachine final : public multipass::VirtualMachine return std::make_unique(); } - SnapshotVista view_snapshots() const noexcept override + SnapshotVista view_snapshots() const override { return {}; } From ea7dcc64e3de9499e0acaaf3bf11d9fef2681f7a Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 12:58:02 +0100 Subject: [PATCH 04/20] [settings] Add noexcept to handler's constructor --- src/daemon/snapshot_settings_handler.cpp | 2 +- src/daemon/snapshot_settings_handler.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/daemon/snapshot_settings_handler.cpp b/src/daemon/snapshot_settings_handler.cpp index 3451a37fc6..0af41669a7 100644 --- a/src/daemon/snapshot_settings_handler.cpp +++ b/src/daemon/snapshot_settings_handler.cpp @@ -80,7 +80,7 @@ mp::SnapshotSettingsException::SnapshotSettingsException(const std::string& deta mp::SnapshotSettingsHandler::SnapshotSettingsHandler( std::unordered_map& operative_instances, const std::unordered_map& deleted_instances, - const std::unordered_set& preparing_instances) + const std::unordered_set& preparing_instances) noexcept : operative_instances{operative_instances}, deleted_instances{deleted_instances}, preparing_instances{preparing_instances} diff --git a/src/daemon/snapshot_settings_handler.h b/src/daemon/snapshot_settings_handler.h index 5eecad7663..c613bbc6ca 100644 --- a/src/daemon/snapshot_settings_handler.h +++ b/src/daemon/snapshot_settings_handler.h @@ -35,7 +35,7 @@ class SnapshotSettingsHandler : public SettingsHandler public: SnapshotSettingsHandler(std::unordered_map& operative_instances, const std::unordered_map& deleted_instances, - const std::unordered_set& preparing_instances); + const std::unordered_set& preparing_instances) noexcept; std::set keys() const override; QString get(const QString& key) const override; From f06550e41358363031c7e275a1c95a1d8e3d93a7 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 13:02:31 +0100 Subject: [PATCH 05/20] [mounts] Add noexcept to new MountHandler accessor --- include/multipass/mount_handler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/multipass/mount_handler.h b/include/multipass/mount_handler.h index 56167d6a49..11e387dbb8 100644 --- a/include/multipass/mount_handler.h +++ b/include/multipass/mount_handler.h @@ -67,7 +67,7 @@ class MountHandler : private DisabledCopyMove active = false; } - const VMMount& get_mount_spec() const + const VMMount& get_mount_spec() const noexcept { return mount_spec; } From 490eb02d68104db5c4aa0ec247b17462289cabb4 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 17:08:51 +0100 Subject: [PATCH 06/20] [vmspecs] Move header to generic include folder --- {src/daemon => include/multipass}/vm_specs.h | 31 ++++++++++++++----- src/daemon/daemon.h | 3 +- src/daemon/instance_settings_handler.h | 2 +- .../backends/shared/base_snapshot.cpp | 2 +- .../backends/shared/base_virtual_machine.cpp | 2 +- tests/json_test_utils.h | 3 +- tests/test_instance_settings_handler.cpp | 2 +- 7 files changed, 29 insertions(+), 16 deletions(-) rename {src/daemon => include/multipass}/vm_specs.h (59%) diff --git a/src/daemon/vm_specs.h b/include/multipass/vm_specs.h similarity index 59% rename from src/daemon/vm_specs.h rename to include/multipass/vm_specs.h index af638990a3..42af152bed 100644 --- a/src/daemon/vm_specs.h +++ b/include/multipass/vm_specs.h @@ -18,10 +18,10 @@ #ifndef MULTIPASS_VM_SPECS_H #define MULTIPASS_VM_SPECS_H -#include -#include -#include -#include +#include "memory_size.h" +#include "network_interface.h" +#include "virtual_machine.h" +#include "vm_mount.h" #include #include @@ -48,10 +48,25 @@ struct VMSpecs inline bool operator==(const VMSpecs& a, const VMSpecs& b) { - return std::tie(a.num_cores, a.mem_size, a.disk_space, a.default_mac_address, a.extra_interfaces, a.ssh_username, - a.state, a.mounts, a.deleted, a.metadata) == - std::tie(b.num_cores, b.mem_size, b.disk_space, b.default_mac_address, b.extra_interfaces, b.ssh_username, - b.state, b.mounts, b.deleted, b.metadata); + return std::tie(a.num_cores, + a.mem_size, + a.disk_space, + a.default_mac_address, + a.extra_interfaces, + a.ssh_username, + a.state, + a.mounts, + a.deleted, + a.metadata) == std::tie(b.num_cores, + b.mem_size, + b.disk_space, + b.default_mac_address, + b.extra_interfaces, + b.ssh_username, + b.state, + b.mounts, + b.deleted, + b.metadata); } inline bool operator!=(const VMSpecs& a, const VMSpecs& b) // TODO drop in C++20 diff --git a/src/daemon/daemon.h b/src/daemon/daemon.h index 4e915a93df..790b1c87fa 100644 --- a/src/daemon/daemon.h +++ b/src/daemon/daemon.h @@ -20,13 +20,12 @@ #include "daemon_config.h" #include "daemon_rpc.h" -#include "multipass/virtual_machine.h" -#include "vm_specs.h" #include #include #include #include +#include #include #include diff --git a/src/daemon/instance_settings_handler.h b/src/daemon/instance_settings_handler.h index 9a17253e1c..9b8692e9b7 100644 --- a/src/daemon/instance_settings_handler.h +++ b/src/daemon/instance_settings_handler.h @@ -18,11 +18,11 @@ #ifndef MULTIPASS_INSTANCE_SETTINGS_HANDLER_H #define MULTIPASS_INSTANCE_SETTINGS_HANDLER_H -#include "vm_specs.h" #include #include #include +#include #include diff --git a/src/platform/backends/shared/base_snapshot.cpp b/src/platform/backends/shared/base_snapshot.cpp index f898b27920..12a0d5af80 100644 --- a/src/platform/backends/shared/base_snapshot.cpp +++ b/src/platform/backends/shared/base_snapshot.cpp @@ -16,12 +16,12 @@ */ #include "base_snapshot.h" -#include "daemon/vm_specs.h" // TODO@snapshots move this #include #include // TODO@snapshots may be able to drop after extracting JSON utilities #include #include +#include #include diff --git a/src/platform/backends/shared/base_virtual_machine.cpp b/src/platform/backends/shared/base_virtual_machine.cpp index f9b97e54ad..39140dad79 100644 --- a/src/platform/backends/shared/base_virtual_machine.cpp +++ b/src/platform/backends/shared/base_virtual_machine.cpp @@ -16,7 +16,6 @@ */ #include "base_virtual_machine.h" -#include "daemon/vm_specs.h" // TODO@snapshots move this #include #include @@ -26,6 +25,7 @@ #include #include #include +#include #include diff --git a/tests/json_test_utils.h b/tests/json_test_utils.h index 1c8df6cf4e..7bd74e73ef 100644 --- a/tests/json_test_utils.h +++ b/tests/json_test_utils.h @@ -21,8 +21,7 @@ #include "temp_dir.h" #include - -#include +#include #include #include diff --git a/tests/test_instance_settings_handler.cpp b/tests/test_instance_settings_handler.cpp index d5954bbe39..12c9eeeeaf 100644 --- a/tests/test_instance_settings_handler.cpp +++ b/tests/test_instance_settings_handler.cpp @@ -20,9 +20,9 @@ #include #include +#include #include -#include #include From 6f46a2fc5a46c28c3da8378107858ac0dd3e4a63 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 17:32:03 +0100 Subject: [PATCH 07/20] [snapshot] Throw on unsupported state --- src/platform/backends/shared/base_snapshot.cpp | 5 ++++- src/platform/backends/shared/base_virtual_machine.cpp | 2 -- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/platform/backends/shared/base_snapshot.cpp b/src/platform/backends/shared/base_snapshot.cpp index 12a0d5af80..ae36135f53 100644 --- a/src/platform/backends/shared/base_snapshot.cpp +++ b/src/platform/backends/shared/base_snapshot.cpp @@ -16,6 +16,7 @@ */ #include "base_snapshot.h" +#include "multipass/virtual_machine.h" #include #include // TODO@snapshots may be able to drop after extracting JSON utilities @@ -146,9 +147,11 @@ mp::BaseSnapshot::BaseSnapshot(const std::string& name, // NOLINT(modernize-p captured{captured} { assert(index > 0 && "snapshot indices need to start at 1"); + using St = VirtualMachine::State; + if (state != St::off && state != St::stopped) + throw std::runtime_error{fmt::format("Unsupported VM state in snapshot: {}", static_cast(state))}; if (index > max_snapshots) throw std::runtime_error{fmt::format("Maximum number of snapshots exceeded: {}", max_snapshots)}; - if (name.empty()) throw std::runtime_error{"Snapshot names cannot be empty"}; if (num_cores < 1) diff --git a/src/platform/backends/shared/base_virtual_machine.cpp b/src/platform/backends/shared/base_virtual_machine.cpp index 39140dad79..79edcbf7fd 100644 --- a/src/platform/backends/shared/base_virtual_machine.cpp +++ b/src/platform/backends/shared/base_virtual_machine.cpp @@ -522,8 +522,6 @@ void BaseVirtualMachine::restore_snapshot(const std::string& name, VMSpecs& spec assert_vm_stopped(state); // precondition auto snapshot = get_snapshot(name); - - // TODO@snapshots convert into runtime_error (persisted info could have been tampered with) assert(snapshot->get_state() == St::off || snapshot->get_state() == St::stopped); snapshot->apply(); From 1848be07241c6e14baa90e27cbd5e46ad194dfc2 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 17:34:34 +0100 Subject: [PATCH 08/20] [snapshot] Throw on non-positive snapshot index --- src/platform/backends/shared/base_snapshot.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/platform/backends/shared/base_snapshot.cpp b/src/platform/backends/shared/base_snapshot.cpp index ae36135f53..2f0a3d5d2f 100644 --- a/src/platform/backends/shared/base_snapshot.cpp +++ b/src/platform/backends/shared/base_snapshot.cpp @@ -150,6 +150,8 @@ mp::BaseSnapshot::BaseSnapshot(const std::string& name, // NOLINT(modernize-p using St = VirtualMachine::State; if (state != St::off && state != St::stopped) throw std::runtime_error{fmt::format("Unsupported VM state in snapshot: {}", static_cast(state))}; + if (index < 1) + throw std::runtime_error{fmt::format("Snapshot index not positive: {}", index)}; if (index > max_snapshots) throw std::runtime_error{fmt::format("Maximum number of snapshots exceeded: {}", max_snapshots)}; if (name.empty()) From 30795e1b00ec394f65de9247f546f41860da607d Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 17:35:48 +0100 Subject: [PATCH 09/20] [snapshot] Fix index overflow exception contents --- src/platform/backends/shared/base_snapshot.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/backends/shared/base_snapshot.cpp b/src/platform/backends/shared/base_snapshot.cpp index 2f0a3d5d2f..51d8bf734e 100644 --- a/src/platform/backends/shared/base_snapshot.cpp +++ b/src/platform/backends/shared/base_snapshot.cpp @@ -153,7 +153,7 @@ mp::BaseSnapshot::BaseSnapshot(const std::string& name, // NOLINT(modernize-p if (index < 1) throw std::runtime_error{fmt::format("Snapshot index not positive: {}", index)}; if (index > max_snapshots) - throw std::runtime_error{fmt::format("Maximum number of snapshots exceeded: {}", max_snapshots)}; + throw std::runtime_error{fmt::format("Maximum number of snapshots exceeded: {}", index)}; if (name.empty()) throw std::runtime_error{"Snapshot names cannot be empty"}; if (num_cores < 1) From 59e7ec4b11319611780544e135b55294dbe80823 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 18:28:12 +0100 Subject: [PATCH 10/20] [vm] Replace TODO with justification Remove a TODO to remove a method that turns out to be justified. This was requested in review but, after discussion, we agreed to drop it. --- include/multipass/virtual_machine.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/multipass/virtual_machine.h b/include/multipass/virtual_machine.h index 1a3716f6a0..c3cc177790 100644 --- a/include/multipass/virtual_machine.h +++ b/include/multipass/virtual_machine.h @@ -99,7 +99,8 @@ class VirtualMachine : private DisabledCopyMove virtual std::shared_ptr take_snapshot(const VMSpecs& specs, const std::string& snapshot_name, const std::string& comment) = 0; - virtual void rename_snapshot(const std::string& old_name, const std::string& new_name) = 0; // TODO@snapshots remove + virtual void rename_snapshot(const std::string& old_name, + const std::string& new_name) = 0; // only VM can avoid repeated names virtual void delete_snapshot(const std::string& name) = 0; virtual void restore_snapshot(const std::string& name, VMSpecs& specs) = 0; virtual void load_snapshots() = 0; From 7f499c6f05cf3618d4521e5a043adc886c3a4bdf Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 18:57:08 +0100 Subject: [PATCH 11/20] [mount] Add a couple of constructors to VMMount In preparation for a constructor to read JSON. --- include/multipass/vm_mount.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/multipass/vm_mount.h b/include/multipass/vm_mount.h index 3f1e911b24..7a442c992c 100644 --- a/include/multipass/vm_mount.h +++ b/include/multipass/vm_mount.h @@ -32,6 +32,15 @@ struct VMMount Native = 1 }; + VMMount() = default; + VMMount(const std::string& sourcePath, id_mappings gidMappings, id_mappings uidMappings, MountType mountType) + : source_path(sourcePath), + gid_mappings(std::move(gidMappings)), + uid_mappings(std::move(uidMappings)), + mount_type(mountType) + { + } + std::string source_path; id_mappings gid_mappings; id_mappings uid_mappings; From e9443e576fe8915f186f7f562fa38ca454c69c64 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 19:02:54 +0100 Subject: [PATCH 12/20] [mount] Move ctor implementation to cpp --- include/multipass/vm_mount.h | 8 +------- src/utils/CMakeLists.txt | 3 ++- src/utils/vm_mount.cpp | 31 +++++++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 8 deletions(-) create mode 100644 src/utils/vm_mount.cpp diff --git a/include/multipass/vm_mount.h b/include/multipass/vm_mount.h index 7a442c992c..0114ed5a1b 100644 --- a/include/multipass/vm_mount.h +++ b/include/multipass/vm_mount.h @@ -33,13 +33,7 @@ struct VMMount }; VMMount() = default; - VMMount(const std::string& sourcePath, id_mappings gidMappings, id_mappings uidMappings, MountType mountType) - : source_path(sourcePath), - gid_mappings(std::move(gidMappings)), - uid_mappings(std::move(uidMappings)), - mount_type(mountType) - { - } + VMMount(const std::string& sourcePath, id_mappings gidMappings, id_mappings uidMappings, MountType mountType); std::string source_path; id_mappings gid_mappings; diff --git a/src/utils/CMakeLists.txt b/src/utils/CMakeLists.txt index ced4a8824b..cd5d0e4d2a 100644 --- a/src/utils/CMakeLists.txt +++ b/src/utils/CMakeLists.txt @@ -21,7 +21,8 @@ function(add_target TARGET_NAME) standard_paths.cpp timer.cpp utils.cpp - vm_image_vault_utils.cpp) + vm_image_vault_utils.cpp + vm_mount.cpp) target_link_libraries(${TARGET_NAME} cert diff --git a/src/utils/vm_mount.cpp b/src/utils/vm_mount.cpp new file mode 100644 index 0000000000..d605485ab7 --- /dev/null +++ b/src/utils/vm_mount.cpp @@ -0,0 +1,31 @@ +/* + * 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 . + * + */ + +#include + +namespace mp = multipass; + +mp::VMMount::VMMount(const std::string& sourcePath, + id_mappings gidMappings, + id_mappings uidMappings, + MountType mountType) + : source_path(sourcePath), + gid_mappings(std::move(gidMappings)), + uid_mappings(std::move(uidMappings)), + mount_type(mountType) +{ +} From 895421d4c77c0789ab5d1052beff53f6185a6335 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 19:17:47 +0100 Subject: [PATCH 13/20] [mount] Construct mounts from json --- include/multipass/vm_mount.h | 3 +++ src/utils/vm_mount.cpp | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/include/multipass/vm_mount.h b/include/multipass/vm_mount.h index 0114ed5a1b..58020f2d12 100644 --- a/include/multipass/vm_mount.h +++ b/include/multipass/vm_mount.h @@ -20,6 +20,8 @@ #include +#include + #include namespace multipass @@ -33,6 +35,7 @@ struct VMMount }; VMMount() = default; + VMMount(const QJsonObject& json); VMMount(const std::string& sourcePath, id_mappings gidMappings, id_mappings uidMappings, MountType mountType); std::string source_path; diff --git a/src/utils/vm_mount.cpp b/src/utils/vm_mount.cpp index d605485ab7..d93b28d03f 100644 --- a/src/utils/vm_mount.cpp +++ b/src/utils/vm_mount.cpp @@ -17,8 +17,38 @@ #include +#include + namespace mp = multipass; +namespace +{ +mp::VMMount parse_json(const QJsonObject& json) +{ + mp::id_mappings uid_mappings; + mp::id_mappings gid_mappings; + auto source_path = json["source_path"].toString().toStdString(); + + for (const QJsonValueRef uid_entry : json["uid_mappings"].toArray()) + { + uid_mappings.push_back( + {uid_entry.toObject()["host_uid"].toInt(), uid_entry.toObject()["instance_uid"].toInt()}); + } + + for (const QJsonValueRef gid_entry : json["gid_mappings"].toArray()) + { + gid_mappings.push_back( + {gid_entry.toObject()["host_gid"].toInt(), gid_entry.toObject()["instance_gid"].toInt()}); + } + + uid_mappings = mp::unique_id_mappings(uid_mappings); + gid_mappings = mp::unique_id_mappings(gid_mappings); + auto mount_type = mp::VMMount::MountType(json["mount_type"].toInt()); + + return mp::VMMount{std::move(source_path), std::move(gid_mappings), std::move(uid_mappings), mount_type}; +} +} // namespace + mp::VMMount::VMMount(const std::string& sourcePath, id_mappings gidMappings, id_mappings uidMappings, @@ -29,3 +59,7 @@ mp::VMMount::VMMount(const std::string& sourcePath, mount_type(mountType) { } + +mp::VMMount::VMMount(const QJsonObject& json) : VMMount{parse_json(json)} // delegate on copy ctor +{ +} From 4a8ef0ba3082f25a6da050f5b02051d04b0f2883 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 19:30:06 +0100 Subject: [PATCH 14/20] [daemon] Use new VMMount constructor from JSON --- src/daemon/daemon.cpp | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index 6f9063fea1..146834e6b1 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -325,30 +325,8 @@ std::unordered_map load_db(const mp::Path& data_path, for (QJsonValueRef entry : record["mounts"].toArray()) { - mp::id_mappings uid_mappings; - mp::id_mappings gid_mappings; - - auto target_path = entry.toObject()["target_path"].toString().toStdString(); - auto source_path = entry.toObject()["source_path"].toString().toStdString(); - - for (QJsonValueRef uid_entry : entry.toObject()["uid_mappings"].toArray()) - { - uid_mappings.push_back( - {uid_entry.toObject()["host_uid"].toInt(), uid_entry.toObject()["instance_uid"].toInt()}); - } - - for (QJsonValueRef gid_entry : entry.toObject()["gid_mappings"].toArray()) - { - gid_mappings.push_back( - {gid_entry.toObject()["host_gid"].toInt(), gid_entry.toObject()["instance_gid"].toInt()}); - } - - uid_mappings = mp::unique_id_mappings(uid_mappings); - gid_mappings = mp::unique_id_mappings(gid_mappings); - auto mount_type = mp::VMMount::MountType(entry.toObject()["mount_type"].toInt()); - - mp::VMMount mount{source_path, gid_mappings, uid_mappings, mount_type}; - mounts[target_path] = mount; + const auto& json = entry.toObject(); + mounts[json["target_path"].toString().toStdString()] = mp::VMMount{json}; } reconstructed_records[key] = {num_cores, From 535622cdb335c54e92d22778a3dad196c3d60ce1 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 19:34:34 +0100 Subject: [PATCH 15/20] [snapshot] Use new VMMount constructor from JSON --- .../backends/shared/base_snapshot.cpp | 31 +++---------------- 1 file changed, 4 insertions(+), 27 deletions(-) diff --git a/src/platform/backends/shared/base_snapshot.cpp b/src/platform/backends/shared/base_snapshot.cpp index 51d8bf734e..8335f253ba 100644 --- a/src/platform/backends/shared/base_snapshot.cpp +++ b/src/platform/backends/shared/base_snapshot.cpp @@ -19,7 +19,6 @@ #include "multipass/virtual_machine.h" #include -#include // TODO@snapshots may be able to drop after extracting JSON utilities #include #include #include @@ -69,35 +68,13 @@ QJsonObject read_snapshot_json(const QString& filename) return json["snapshot"].toObject(); } -std::unordered_map load_mounts(const QJsonArray& json) +std::unordered_map load_mounts(const QJsonArray& mounts_json) { std::unordered_map mounts; - for (const auto& entry : json) + for (const auto& entry : mounts_json) { - mp::id_mappings uid_mappings; - mp::id_mappings gid_mappings; - - auto target_path = entry.toObject()["target_path"].toString().toStdString(); - auto source_path = entry.toObject()["source_path"].toString().toStdString(); - - for (const QJsonValueRef uid_entry : entry.toObject()["uid_mappings"].toArray()) - { - uid_mappings.push_back( - {uid_entry.toObject()["host_uid"].toInt(), uid_entry.toObject()["instance_uid"].toInt()}); - } - - for (const QJsonValueRef gid_entry : entry.toObject()["gid_mappings"].toArray()) - { - gid_mappings.push_back( - {gid_entry.toObject()["host_gid"].toInt(), gid_entry.toObject()["instance_gid"].toInt()}); - } - - uid_mappings = mp::unique_id_mappings(uid_mappings); - gid_mappings = mp::unique_id_mappings(gid_mappings); - auto mount_type = mp::VMMount::MountType(entry.toObject()["mount_type"].toInt()); - - mp::VMMount mount{source_path, gid_mappings, uid_mappings, mount_type}; - mounts[target_path] = std::move(mount); + const auto& json = entry.toObject(); + mounts[json["target_path"].toString().toStdString()] = mp::VMMount{json}; } return mounts; From deb31ee963ae6a5b9b1e13e7beb96bf310d6ebf8 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 19:51:05 +0100 Subject: [PATCH 16/20] [mount] Serialize mounts to JSON --- include/multipass/vm_mount.h | 2 ++ src/utils/vm_mount.cpp | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/include/multipass/vm_mount.h b/include/multipass/vm_mount.h index 58020f2d12..b3f6b37eff 100644 --- a/include/multipass/vm_mount.h +++ b/include/multipass/vm_mount.h @@ -38,6 +38,8 @@ struct VMMount VMMount(const QJsonObject& json); VMMount(const std::string& sourcePath, id_mappings gidMappings, id_mappings uidMappings, MountType mountType); + QJsonObject serialize() const; + std::string source_path; id_mappings gid_mappings; id_mappings uid_mappings; diff --git a/src/utils/vm_mount.cpp b/src/utils/vm_mount.cpp index d93b28d03f..b798eed92b 100644 --- a/src/utils/vm_mount.cpp +++ b/src/utils/vm_mount.cpp @@ -63,3 +63,38 @@ mp::VMMount::VMMount(const std::string& sourcePath, mp::VMMount::VMMount(const QJsonObject& json) : VMMount{parse_json(json)} // delegate on copy ctor { } + +QJsonObject mp::VMMount::serialize() const +{ + QJsonObject ret; + ret.insert("source_path", QString::fromStdString(source_path)); + + QJsonArray uid_mappings_json; + + for (const auto& map : uid_mappings) + { + QJsonObject map_entry; + map_entry.insert("host_uid", map.first); + map_entry.insert("instance_uid", map.second); + + uid_mappings_json.append(map_entry); + } + + ret.insert("uid_mappings", uid_mappings_json); + + QJsonArray gid_mappings_json; + + for (const auto& map : gid_mappings) + { + QJsonObject map_entry; + map_entry.insert("host_gid", map.first); + map_entry.insert("instance_gid", map.second); + + gid_mappings_json.append(map_entry); + } + + ret.insert("gid_mappings", gid_mappings_json); + + ret.insert("mount_type", static_cast(mount_type)); + return ret; +} From a5490c3fe95282b564d39b5fbfe42adf21dda768 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 19:55:05 +0100 Subject: [PATCH 17/20] [daemon] Use new VMMount serialize method --- src/daemon/daemon.cpp | 31 +------------------------------ 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index 146834e6b1..baec99d77b 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -378,37 +378,8 @@ QJsonObject vm_spec_to_json(const mp::VMSpecs& specs) QJsonArray json_mounts; for (const auto& mount : specs.mounts) { - QJsonObject entry; - entry.insert("source_path", QString::fromStdString(mount.second.source_path)); + auto entry = mount.second.serialize(); entry.insert("target_path", QString::fromStdString(mount.first)); - - QJsonArray uid_mappings; - - for (const auto& map : mount.second.uid_mappings) - { - QJsonObject map_entry; - map_entry.insert("host_uid", map.first); - map_entry.insert("instance_uid", map.second); - - uid_mappings.append(map_entry); - } - - entry.insert("uid_mappings", uid_mappings); - - QJsonArray gid_mappings; - - for (const auto& map : mount.second.gid_mappings) - { - QJsonObject map_entry; - map_entry.insert("host_gid", map.first); - map_entry.insert("instance_gid", map.second); - - gid_mappings.append(map_entry); - } - - entry.insert("gid_mappings", gid_mappings); - - entry.insert("mount_type", static_cast(mount.second.mount_type)); json_mounts.append(entry); } From 492019e2feffea24d53ece1864fa0bae92999558 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 19:59:04 +0100 Subject: [PATCH 18/20] [snapshot] Use new VMMount serialize method --- .../backends/shared/base_snapshot.cpp | 31 +------------------ 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/src/platform/backends/shared/base_snapshot.cpp b/src/platform/backends/shared/base_snapshot.cpp index 8335f253ba..0c86e48ac0 100644 --- a/src/platform/backends/shared/base_snapshot.cpp +++ b/src/platform/backends/shared/base_snapshot.cpp @@ -206,37 +206,8 @@ QJsonObject mp::BaseSnapshot::serialize() const QJsonArray json_mounts; for (const auto& mount : mounts) { - QJsonObject entry; - entry.insert("source_path", QString::fromStdString(mount.second.source_path)); + auto entry = mount.second.serialize(); entry.insert("target_path", QString::fromStdString(mount.first)); - - QJsonArray uid_mappings; - - for (const auto& map : mount.second.uid_mappings) - { - QJsonObject map_entry; - map_entry.insert("host_uid", map.first); - map_entry.insert("instance_uid", map.second); - - uid_mappings.append(map_entry); - } - - entry.insert("uid_mappings", uid_mappings); - - QJsonArray gid_mappings; - - for (const auto& map : mount.second.gid_mappings) - { - QJsonObject map_entry; - map_entry.insert("host_gid", map.first); - map_entry.insert("instance_gid", map.second); - - gid_mappings.append(map_entry); - } - - entry.insert("gid_mappings", gid_mappings); - - entry.insert("mount_type", static_cast(mount.second.mount_type)); json_mounts.append(entry); } From e0f936570b20992fbc819f3cd7fa45d4bd5cb4f0 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 20 Oct 2023 20:01:22 +0100 Subject: [PATCH 19/20] [snapshot] Remove incorrect TODO --- src/platform/backends/shared/base_snapshot.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/backends/shared/base_snapshot.cpp b/src/platform/backends/shared/base_snapshot.cpp index 0c86e48ac0..c5b963894c 100644 --- a/src/platform/backends/shared/base_snapshot.cpp +++ b/src/platform/backends/shared/base_snapshot.cpp @@ -25,7 +25,7 @@ #include -#include // TODO@snapshots may be able to drop after extracting JSON utilities +#include #include #include From f8828219c70081a1bedfbfe5f290fa8036575cd3 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 25 Oct 2023 11:58:08 +0100 Subject: [PATCH 20/20] [cosmetic] Fix extra newline --- src/daemon/instance_settings_handler.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/daemon/instance_settings_handler.h b/src/daemon/instance_settings_handler.h index 9b8692e9b7..d2236be533 100644 --- a/src/daemon/instance_settings_handler.h +++ b/src/daemon/instance_settings_handler.h @@ -18,7 +18,6 @@ #ifndef MULTIPASS_INSTANCE_SETTINGS_HANDLER_H #define MULTIPASS_INSTANCE_SETTINGS_HANDLER_H - #include #include #include