Merge #2835

2835: Mount handlers refactor r=ricab,luis4a0,sharder996,townsend2010 a=andrei-toterman This PR is a refactor of the mount handlers. Instead of having each mount handler keep track of all the mounts of a specific type, we will have each instance of a mount handler class represent a single mounted path. The daemon will keep track of these mounts and, since each mount can be thought of as a resource, we can use RAII to manage their creation and destruction to some extent. Previous tests have been removed in order to not block the next release, but they will be rewritten as soon as possible. Co-authored-by: Andrei Toterman <[email protected]>
canonical · Jan 11, 2023 · 9bcabbf · 9bcabbf
1 parent 66e98b5
commit 9bcabbf
Show file tree

Hide file tree

Showing 39 changed files with 369 additions and 2,067 deletions.
diff --git a/include/multipass/mount_handler.h b/include/multipass/mount_handler.h
@@ -18,20 +18,17 @@
 #ifndef MULTIPASS_MOUNT_HANDLER_H
 #define MULTIPASS_MOUNT_HANDLER_H
 
-#include <multipass/disabled_copy_move.h>
-#include <multipass/id_mappings.h>
+#include <multipass/file_ops.h>
 #include <multipass/rpc/multipass.grpc.pb.h>
 #include <multipass/ssh/ssh_key_provider.h>
+#include <multipass/vm_mount.h>
 
 #include <chrono>
-#include <memory>
-#include <string>
 #include <variant>
 
 namespace multipass
 {
 class VirtualMachine;
-struct VMMount;
 
 using ServerVariant = std::variant<grpc::ServerReaderWriterInterface<StartReply, StartRequest>*,
                                    grpc::ServerReaderWriterInterface<LaunchReply, LaunchRequest>*,
@@ -45,31 +42,65 @@ class MountHandler : private DisabledCopyMove
 
     virtual ~MountHandler() = default;
 
-    // Used to set up anything host side related
-    virtual void init_mount(VirtualMachine* vm, const std::string& target_path, const VMMount& vm_mount) = 0;
-    virtual void start_mount(VirtualMachine* vm, ServerVariant server, const std::string& target_path,
-                             const std::chrono::milliseconds& timeout = std::chrono::minutes(5)) = 0;
-    virtual void stop_mount(const std::string& instance, const std::string& path) = 0;
-    virtual void stop_all_mounts_for_instance(const std::string& instance) = 0;
-    virtual bool has_instance_already_mounted(const std::string& instance, const std::string& path) const = 0;
+    void start(ServerVariant server, std::chrono::milliseconds timeout = std::chrono::minutes(5))
+    {
+        std::lock_guard active_lock{active_mutex};
+        if (active)
+            return;
+        start_impl(server, timeout);
+        active = true;
+    }
+
+    void stop(bool force = false)
+    {
+        std::lock_guard active_lock{active_mutex};
+        if (!active)
+            return;
+        stop_impl(force);
+        active = false;
+    }
 
 protected:
-    MountHandler(const SSHKeyProvider& ssh_key_provider) : ssh_key_provider(&ssh_key_provider){};
+    MountHandler(VirtualMachine* vm, const SSHKeyProvider* ssh_key_provider, const std::string& target,
+                 const VMMount& mount)
+        : vm{vm}, ssh_key_provider{ssh_key_provider}, target{target}, active{false}
+    {
+        std::error_code err;
+        auto source_status = MP_FILEOPS.status(mount.source_path, err);
+        if (source_status.type() == fs::file_type::not_found)
+            throw std::runtime_error(fmt::format("Mount source path \"{}\" does not exist.", mount.source_path));
+        if (err)
+            throw std::runtime_error(
+                fmt::format("Mount source path \"{}\" is not accessible: {}.", mount.source_path, err.message()));
+        if (source_status.type() != fs::file_type::directory)
+            throw std::runtime_error(fmt::format("Mount source path \"{}\" is not a directory.", mount.source_path));
+        if (source_status.permissions() != fs::perms::unknown &&
+            (source_status.permissions() & fs::perms::owner_read) == fs::perms::none)
+            throw std::runtime_error(fmt::format("Mount source path \"{}\" is not readable.", mount.source_path));
+    };
+
+    virtual void start_impl(ServerVariant server, std::chrono::milliseconds timeout) = 0;
+    virtual void stop_impl(bool force) = 0;
 
     template <typename Reply, typename Request>
-    Reply make_reply_from_server(grpc::ServerReaderWriterInterface<Reply, Request>&)
+    static Reply make_reply_from_server(grpc::ServerReaderWriterInterface<Reply, Request>*)
     {
         return Reply{};
     }
 
     template <typename Reply, typename Request>
-    Request make_request_from_server(grpc::ServerReaderWriterInterface<Reply, Request>&)
+    static Request make_request_from_server(grpc::ServerReaderWriterInterface<Reply, Request>*)
     {
         return Request{};
     }
 
+    VirtualMachine* vm;
     const SSHKeyProvider* ssh_key_provider;
-};
+    const std::string target;
 
+private:
+    bool active;
+    std::mutex active_mutex;
+};
 } // namespace multipass
 #endif // MULTIPASS_MOUNT_HANDLER_H
diff --git a/include/multipass/sshfs_mount/sshfs_mount_handler.h b/include/multipass/sshfs_mount/sshfs_mount_handler.h
@@ -20,37 +20,23 @@
 
 #include <multipass/mount_handler.h>
 #include <multipass/process/process.h>
-#include <multipass/qt_delete_later_unique_ptr.h>
-#include <multipass/ssh/ssh_key_provider.h>
 #include <multipass/sshfs_server_config.h>
 
-#include <unordered_map>
-
 namespace multipass
 {
-class VirtualMachine;
-struct VMMount;
-
-class SSHFSMountHandler : public QObject, public MountHandler
+class SSHFSMountHandler : public MountHandler
 {
-    Q_OBJECT
 public:
-    explicit SSHFSMountHandler(const SSHKeyProvider& ssh_key_provider);
-
-    void init_mount(VirtualMachine* vm, const std::string& target_path, const VMMount& vm_mount) override;
-    void start_mount(VirtualMachine* vm, ServerVariant server, const std::string& target_path,
-                     const std::chrono::milliseconds& timeout = std::chrono::minutes(5)) override;
+    SSHFSMountHandler(VirtualMachine* vm, const SSHKeyProvider* ssh_key_provider, const std::string& target,
+                      const VMMount& mount);
+    ~SSHFSMountHandler() override;
 
-    void stop_mount(const std::string& instance, const std::string& path) override;
-    void stop_all_mounts_for_instance(const std::string& instance) override;
-
-    bool has_instance_already_mounted(const std::string& instance, const std::string& path) const override;
+    void start_impl(ServerVariant server, std::chrono::milliseconds timeout) override;
+    void stop_impl(bool force) override;
 
 private:
-    std::unordered_map<std::string, std::unordered_map<std::string, SSHFSServerConfig>> sshfs_server_configs;
-    std::unordered_map<std::string, std::unordered_map<std::string, qt_delete_later_unique_ptr<Process>>>
-        mount_processes;
+    Process::UPtr process;
+    SSHFSServerConfig config;
 };
-
 } // namespace multipass
 #endif // MULTIPASS_SSHFS_MOUNT_HANDLER_H
diff --git a/include/multipass/virtual_machine.h b/include/multipass/virtual_machine.h
@@ -34,6 +34,7 @@ namespace multipass
 class MemorySize;
 class SSHKeyProvider;
 struct VMMount;
+class MountHandler;
 
 class VirtualMachine : private DisabledCopyMove
 {
@@ -76,8 +77,9 @@ class VirtualMachine : private DisabledCopyMove
     virtual void update_cpus(int num_cores) = 0;
     virtual void resize_memory(const MemorySize& new_size) = 0;
     virtual void resize_disk(const MemorySize& new_size) = 0;
-    virtual void add_vm_mount(const std::string& target_path, const VMMount& vm_mount) = 0;
-    virtual void delete_vm_mount(const std::string& target_path) = 0;
+    virtual std::unique_ptr<MountHandler> make_native_mount_handler(const SSHKeyProvider* ssh_key_provider,
+                                                                    const std::string& target,
+                                                                    const VMMount& mount) = 0;
 
     VirtualMachine::State state;
     const std::string vm_name;

diff --git a/include/multipass/virtual_machine_factory.h b/include/multipass/virtual_machine_factory.h
@@ -33,7 +33,6 @@ class Node;
 
 namespace multipass
 {
-class MountHandler;
 class URLDownloader;
 class VirtualMachineDescription;
 class VMImageHost;
@@ -70,9 +69,6 @@ class VirtualMachineFactory : private DisabledCopyMove
     // List all the network interfaces seen by the backend.
     virtual std::vector<NetworkInterfaceInfo> networks() const = 0;
 
-    // Create the performance mount handler for the backend.
-    virtual std::unique_ptr<MountHandler> create_performance_mount_handler(const SSHKeyProvider& ssh_key_provider) = 0;
-
 protected:
     VirtualMachineFactory() = default;