[snapshots] resolve instance dir in vm

include/multipass/utils.h

@@ -154,6 +154,8 @@ class Utils : public Singleton<Utils>
     // virtual machine helpers
     virtual void wait_for_cloud_init(VirtualMachine* virtual_machine, std::chrono::milliseconds timeout,
                                      const SSHKeyProvider& key_provider) const;
+    virtual Path derive_instances_dir(const Path& data_dir, const Path& backend_directory_name,
+                                      const Path& instances_subdir) const;
 
     // system info helpers
     virtual std::string get_kernel_version() const;

include/multipass/virtual_machine_factory.h

@@ -60,8 +60,7 @@ class VirtualMachineFactory : private DisabledCopyMove
     virtual void prepare_instance_image(const VMImage& instance_image, const VirtualMachineDescription& desc) = 0;
     virtual void hypervisor_health_check() = 0;
     virtual QString get_backend_directory_name() const = 0;
-    virtual Path
-    get_instance_directory_name(const std::string& name) const = 0; // postcondition: return directory exists
+    virtual Path get_instance_directory(const std::string& name) const = 0;
     virtual QString get_backend_version_string() const = 0;
     virtual VMImageVault::UPtr create_image_vault(std::vector<VMImageHost*> image_hosts, URLDownloader* downloader,
                                                   const Path& cache_dir_path, const Path& data_dir_path,

src/daemon/daemon.cpp

@@ -445,7 +445,7 @@ auto fetch_image_for(const std::string& name, mp::VirtualMachineFactory& factory
     mp::Query query{name, "", false, "", mp::Query::Type::Alias, false};
 
     return vault.fetch_image(factory.fetch_type(), query, stub_prepare, stub_progress, false, std::nullopt,
-                             factory.get_instance_directory_name(name));
+                             factory.get_instance_directory(name));
 }
 
 auto try_mem_size(const std::string& val) -> std::optional<mp::MemorySize>
@@ -2917,7 +2917,7 @@ void mp::Daemon::create_vm(const CreateRequest* request,
 
             auto vm_image =
                 config->vault->fetch_image(fetch_type, query, prepare_action, progress_monitor, launch_from_blueprint,
-                                           checksum, config->factory->get_instance_directory_name(name));
+                                           checksum, config->factory->get_instance_directory(name));
 
             const auto image_size = config->vault->minimum_image_size_for(vm_image.id);
             vm_desc.disk_space = compute_final_image_size(

src/daemon/default_vm_image_vault.cpp

@@ -254,7 +254,7 @@ mp::DefaultVMImageVault::~DefaultVMImageVault()
 mp::VMImage mp::DefaultVMImageVault::fetch_image(const FetchType& fetch_type, const Query& query,
                                                  const PrepareAction& prepare, const ProgressMonitor& monitor,
                                                  const bool unlock, const std::optional<std::string>& checksum,
-                                                 const mp::Path& download_dir)
+                                                 const mp::Path& save_dir)
 {
     {
         std::lock_guard<decltype(fetch_mutex)> lock{fetch_mutex};
@@ -282,11 +282,11 @@ mp::VMImage mp::DefaultVMImageVault::fetch_image(const FetchType& fetch_type, co
 
         if (source_image.image_path.endsWith(".xz"))
         {
-            source_image.image_path = extract_image_from(query.name, source_image, monitor, download_dir);
+            source_image.image_path = extract_image_from(query.name, source_image, monitor, save_dir);
         }
         else
         {
-            source_image = image_instance_from(query.name, source_image, download_dir);
+            source_image = image_instance_from(query.name, source_image, save_dir);
         }
 
         vm_image = prepare(source_image);
@@ -327,7 +327,7 @@ mp::VMImage mp::DefaultVMImageVault::fetch_image(const FetchType& fetch_type, co
 
                 if (last_modified.isValid() && (last_modified.toString().toStdString() == record.image.release_date))
                 {
-                    return finalize_image_records(query, record.image, id, download_dir);
+                    return finalize_image_records(query, record.image, id, save_dir);
                 }
             }
 
@@ -389,7 +389,7 @@ mp::VMImage mp::DefaultVMImageVault::fetch_image(const FetchType& fetch_type, co
                         const auto prepared_image = record.second.image;
                         try
                         {
-                            return finalize_image_records(query, prepared_image, record.first, download_dir);
+                            return finalize_image_records(query, prepared_image, record.first, save_dir);
                         }
                         catch (const std::exception& e)
                         {
@@ -427,7 +427,7 @@ mp::VMImage mp::DefaultVMImageVault::fetch_image(const FetchType& fetch_type, co
             auto prepared_image = future.result();
             std::lock_guard<decltype(fetch_mutex)> lock{fetch_mutex};
             in_progress_image_fetches.erase(id);
-            return finalize_image_records(query, prepared_image, id, download_dir);
+            return finalize_image_records(query, prepared_image, id, save_dir);
         }
         catch (const std::exception&)
         {

src/platform/backends/libvirt/libvirt_virtual_machine_factory.cpp

@@ -108,7 +108,8 @@ auto make_libvirt_wrapper(const std::string& libvirt_object_path)
 
 mp::LibVirtVirtualMachineFactory::LibVirtVirtualMachineFactory(const mp::Path& data_dir,
                                                                const std::string& libvirt_object_path)
-    : BaseVirtualMachineFactory(QDir(data_dir, get_backend_directory_name()).filePath("vault/instances")),
+    : BaseVirtualMachineFactory(
+          MP_UTILS.derive_instances_dir(data_dir, get_backend_directory_name(), instances_subdir)),
       libvirt_wrapper{make_libvirt_wrapper(libvirt_object_path)},
       data_dir{data_dir},
       bridge_name{enable_libvirt_network(data_dir, libvirt_wrapper)},
@@ -128,7 +129,7 @@ mp::VirtualMachine::UPtr mp::LibVirtVirtualMachineFactory::create_virtual_machin
         bridge_name = enable_libvirt_network(data_dir, libvirt_wrapper);
 
     return std::make_unique<mp::LibVirtVirtualMachine>(desc, bridge_name, monitor, libvirt_wrapper,
-                                                       get_instance_directory_name(desc.vm_name));
+                                                       get_instance_directory(desc.vm_name));
 }
 
 mp::LibVirtVirtualMachineFactory::~LibVirtVirtualMachineFactory()

src/platform/backends/lxd/lxd_virtual_machine_factory.cpp

@@ -82,12 +82,12 @@ mp::NetworkInterfaceInfo munch_network(std::map<std::string, mp::NetworkInterfac
 
     return ret;
 }
-
 } // namespace
 
 mp::LXDVirtualMachineFactory::LXDVirtualMachineFactory(NetworkAccessManager::UPtr manager, const mp::Path& data_dir,
                                                        const QUrl& base_url)
-    : BaseVirtualMachineFactory(MP_UTILS.make_dir(data_dir, get_backend_directory_name())),
+    : BaseVirtualMachineFactory(
+          MP_UTILS.derive_instances_dir(data_dir, get_backend_directory_name(), instances_subdir)),
       manager{std::move(manager)},
       base_url{base_url}
 {
@@ -103,12 +103,12 @@ mp::VirtualMachine::UPtr mp::LXDVirtualMachineFactory::create_virtual_machine(co
 {
     return std::make_unique<mp::LXDVirtualMachine>(desc, monitor, manager.get(), base_url, multipass_bridge_name,
                                                    storage_pool,
-                                                   MP_UTILS.make_dir(get_instance_directory_name(desc.vm_name)));
+                                                   MP_UTILS.make_dir(get_instance_directory(desc.vm_name)));
 }
 
 void mp::LXDVirtualMachineFactory::remove_resources_for_impl(const std::string& name)
 {
-    mpl::log(mpl::Level::trace, category, fmt::format("No resources to remove for \"{}\"", name));
+    mpl::log(mpl::Level::trace, category, fmt::format("No further resources to remove for \"{}\"", name));
 }
 
 auto mp::LXDVirtualMachineFactory::prepare_source_image(const VMImage& source_image) -> VMImage

src/platform/backends/lxd/lxd_virtual_machine_factory.h

@@ -54,8 +54,6 @@ class LXDVirtualMachineFactory : public BaseVirtualMachineFactory
 
 protected:
     void remove_resources_for_impl(const std::string& name) override;
-
-protected:
     std::string create_bridge_with(const NetworkInterfaceInfo& interface) override;
 
 private:

src/platform/backends/qemu/qemu_virtual_machine_factory.cpp

@@ -34,10 +34,6 @@ namespace mpl = multipass::logging;
 namespace
 {
 constexpr auto category = "qemu factory";
-mp::Path derive_instances_dir(const mp::QemuPlatform& qemu_platform, const mp::Path& data_dir)
-{
-    return QDir(data_dir, qemu_platform.get_directory_name()).filePath("vault/instances");
-}
 } // namespace
 
 mp::QemuVirtualMachineFactory::QemuVirtualMachineFactory(const mp::Path& data_dir)
@@ -46,15 +42,17 @@ mp::QemuVirtualMachineFactory::QemuVirtualMachineFactory(const mp::Path& data_di
 }
 
 mp::QemuVirtualMachineFactory::QemuVirtualMachineFactory(QemuPlatform::UPtr qemu_platform, const mp::Path& data_dir)
-    : BaseVirtualMachineFactory(derive_instances_dir(*qemu_platform, data_dir)), qemu_platform{std::move(qemu_platform)}
+    : BaseVirtualMachineFactory(
+          MP_UTILS.derive_instances_dir(data_dir, qemu_platform->get_directory_name(), instances_subdir)),
+      qemu_platform{std::move(qemu_platform)}
 {
 }
 
 mp::VirtualMachine::UPtr mp::QemuVirtualMachineFactory::create_virtual_machine(const VirtualMachineDescription& desc,
                                                                                VMStatusMonitor& monitor)
 {
     return std::make_unique<mp::QemuVirtualMachine>(desc, qemu_platform.get(), monitor,
-                                                    MP_UTILS.make_dir(get_instance_directory_name(desc.vm_name)));
+                                                    get_instance_directory(desc.vm_name));
 }
 
 void mp::QemuVirtualMachineFactory::remove_resources_for_impl(const std::string& name)

src/platform/backends/shared/base_virtual_machine_factory.cpp

@@ -39,6 +39,8 @@ auto find_bridge_with(const NetworkContainer& networks, const std::string& membe
 }
 } // namespace
 
+const mp::Path mp::BaseVirtualMachineFactory::instances_subdir = "vault/instances";
+
 mp::BaseVirtualMachineFactory::BaseVirtualMachineFactory(const Path& instances_dir) : instances_dir{instances_dir} {};
 
 void mp::BaseVirtualMachineFactory::configure(VirtualMachineDescription& vm_desc)

src/platform/backends/shared/base_virtual_machine_factory.h

@@ -48,7 +48,7 @@ class BaseVirtualMachineFactory : public VirtualMachineFactory
         return {};
     };
 
-    Path get_instance_directory_name(const std::string& name) const override
+    Path get_instance_directory(const std::string& name) const override
     {
         return multipass::utils::backend_directory_path(instances_dir, QString::fromStdString(name));
     }
@@ -73,6 +73,9 @@ class BaseVirtualMachineFactory : public VirtualMachineFactory
         throw NotImplementedOnThisBackendException("networks");
     };
 
+protected:
+    static const Path instances_subdir;
+
 protected:
     std::string create_bridge_with(const NetworkInterfaceInfo& interface) override
     {
@@ -95,7 +98,7 @@ class BaseVirtualMachineFactory : public VirtualMachineFactory
 inline void multipass::BaseVirtualMachineFactory::remove_resources_for(const std::string& name)
 {
     remove_resources_for_impl(name);
-    QDir instance_dir{get_instance_directory_name(name)};
+    QDir instance_dir{get_instance_directory(name)};
     instance_dir.removeRecursively();
 }
 

src/utils/utils.cpp

@@ -674,3 +674,12 @@ void mp::utils::set_owner_for(mp::SSHSession& session, const std::string& root,
                                   fmt::format("sudo /bin/bash -c 'cd \"{}\" && chown -R {}:{} \"{}\"'", root, vm_user,
                                               vm_group, relative_target.substr(0, relative_target.find_first_of('/'))));
 }
+
+mp::Path mp::Utils::derive_instances_dir(const mp::Path& data_dir, const mp::Path& backend_directory_name,
+                                         const mp::Path& instances_subdir) const
+{
+    if (backend_directory_name.isEmpty())
+        return QDir(data_dir).filePath(instances_subdir);
+    else
+        return QDir(data_dir).filePath(QString("%1/%2").arg(backend_directory_name).arg(instances_subdir));
+}

tests/blueprint_test_lambdas.cpp

@@ -41,7 +41,7 @@ mpt::fetch_image_lambda(const std::string& release, const std::string& remote, c
     return [&release, &remote, must_have_checksum](
                const mp::FetchType& fetch_type, const mp::Query& query, const mp::VMImageVault::PrepareAction& prepare,
                const mp::ProgressMonitor& monitor, const bool unlock, const std::optional<std::string>& checksum,
-               const mp::Path& download_dir) {
+               const mp::Path& save_dir) {
         EXPECT_EQ(query.release, release);
         if (remote.empty())
         {
@@ -57,7 +57,7 @@ mpt::fetch_image_lambda(const std::string& release, const std::string& remote, c
             EXPECT_NE(checksum, std::nullopt);
         }
 
-        return mpt::StubVMImageVault().fetch_image(fetch_type, query, prepare, monitor, unlock, checksum, download_dir);
+        return mpt::StubVMImageVault().fetch_image(fetch_type, query, prepare, monitor, unlock, checksum, save_dir);
     };
 }
 

tests/lxd/test_lxd_backend.cpp

@@ -958,10 +958,10 @@ TEST_F(LXDBackend, unimplemented_functions_logs_trace_message)
 
     const std::string name{"foo"};
 
-    EXPECT_CALL(
-        *logger_scope.mock_logger,
-        log(Eq(mpl::Level::trace), mpt::MockLogger::make_cstring_matcher(StrEq("lxd factory")),
-            mpt::MockLogger::make_cstring_matcher(StrEq(fmt::format("No resources to remove for \"{}\"", name)))));
+    EXPECT_CALL(*logger_scope.mock_logger,
+                log(Eq(mpl::Level::trace), mpt::MockLogger::make_cstring_matcher(StrEq("lxd factory")),
+                    mpt::MockLogger::make_cstring_matcher(
+                        StrEq(fmt::format("No further resources to remove for \"{}\"", name)))));
 
     EXPECT_CALL(*logger_scope.mock_logger,
                 log(Eq(mpl::Level::trace), mpt::MockLogger::make_cstring_matcher(StrEq("lxd factory")),