canonical · bors · Aug 7, 2019 · Jul 30, 2019 · Jul 31, 2019
diff --git a/include/multipass/sshfs_mount/sftp_server.h b/include/multipass/sshfs_mount/sftp_server.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017-2018 Canonical, Ltd.
+ * Copyright (C) 2017-2019 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
@@ -31,19 +31,23 @@
 namespace multipass
 {
 class SSHSession;
+class SSHProcess;
+
 class SftpServer
 {
 public:
-    SftpServer(SSHSession&& ssh_session, SSHProcess&& sshfs_proc, const std::string& source,
+    SftpServer(SSHSession&& ssh_session, const std::string& source, const std::string& target,
                const std::unordered_map<int, int>& gid_map, const std::unordered_map<int, int>& uid_map,
                int default_uid, int default_gid);
     SftpServer(SftpServer&& other);
+    ~SftpServer();
 
     void run();
     void stop();
 
     using SSHSessionUptr = std::unique_ptr<ssh_session_struct, decltype(ssh_free)*>;
     using SftpSessionUptr = std::unique_ptr<sftp_session_struct, decltype(sftp_free)*>;
+    using SSHFSProcUptr = std::unique_ptr<SSHProcess>;
 
 private:
     void process_message(sftp_client_message msg);
@@ -70,14 +74,17 @@ class SftpServer
     int handle_extended(sftp_client_message msg);
 
     SSHSession ssh_session;
-    const SftpSessionUptr sftp_server_session;
+    SSHFSProcUptr sshfs_process;
+    SftpSessionUptr sftp_server_session;
     const std::string source_path;
+    const std::string target_path;
     std::unordered_map<void*, std::unique_ptr<QFileInfoList>> open_dir_handles;
     std::unordered_map<void*, std::unique_ptr<QFile>> open_file_handles;
     const std::unordered_map<int, int> gid_map;
     const std::unordered_map<int, int> uid_map;
     const int default_uid;
     const int default_gid;
+    bool stop_invoked{false};
 };
 } // namespace multipass
 #endif // MULTIPASS_SFTP_SERVER_H
diff --git a/src/ssh/ssh_process.cpp b/src/ssh/ssh_process.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017-2018 Canonical, Ltd.
+ * Copyright (C) 2017-2019 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
@@ -84,6 +84,7 @@ mp::SSHProcess::SSHProcess(ssh_session session, const std::string& cmd)
 
 int mp::SSHProcess::exit_code(std::chrono::milliseconds timeout)
 {
+    exit_status = nullopt;
     ExitStatusCallback cb{channel.get(), exit_status};
 
     std::unique_ptr<ssh_event_struct, decltype(ssh_event_free)*> event{ssh_event_new(), ssh_event_free};

diff --git a/src/sshfs_mount/sftp_server.cpp b/src/sshfs_mount/sftp_server.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017-2018 Canonical, Ltd.
+ * Copyright (C) 2017-2019 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
@@ -18,10 +18,12 @@
 #include <multipass/sshfs_mount/sftp_server.h>
 
 #include <multipass/cli/client_platform.h>
+#include <multipass/exceptions/exitless_sshprocess_exception.h>
 #include <multipass/logging/log.h>
 #include <multipass/platform.h>
 #include <multipass/ssh/ssh_session.h>
 #include <multipass/ssh/throw_on_error.h>
+#include <multipass/utils.h>
 
 #include <multipass/format.h>
 
@@ -36,6 +38,7 @@ namespace
 {
 constexpr auto category = "sftp server";
 using SftpHandleUPtr = std::unique_ptr<ssh_string_struct, void (*)(ssh_string)>;
+using namespace std::literals::chrono_literals;
 
 enum Permissions
 {
@@ -225,21 +228,52 @@ auto handle_from(sftp_client_message msg, const std::unordered_map<void*, std::u
         return entry->second.get();
     return nullptr;
 }
+
+void check_sshfs_status(mp::SSHSession& session, mp::SSHProcess& sshfs_process)
+{
+    try
+    {
+        if (sshfs_process.exit_code(250ms) != 0)
+            throw std::runtime_error(sshfs_process.read_std_error());
+    }
+    catch (const mp::ExitlessSSHProcessException&)
+    {
+        // Timeout getting exit status; assume sshfs is running in the instance
+    }
+}
+
+auto create_sshfs_process(mp::SSHSession& session, const std::string& source, const std::string& target)
+{
+    auto sshfs_process = session.exec(fmt::format(
+        "sudo sshfs -o slave -o nonempty -o transform_symlinks -o allow_other :\"{}\" \"{}\"", source, target));
+
+    check_sshfs_status(session, sshfs_process);
+
+    return std::make_unique<mp::SSHProcess>(std::move(sshfs_process));
+}
 } // namespace
 
-mp::SftpServer::SftpServer(SSHSession&& session, SSHProcess&& sshfs_proc, const std::string& source,
+mp::SftpServer::SftpServer(SSHSession&& session, const std::string& source, const std::string& target,
                            const std::unordered_map<int, int>& gid_map, const std::unordered_map<int, int>& uid_map,
                            int default_uid, int default_gid)
     : ssh_session{std::move(session)},
-      sftp_server_session{make_sftp_session(ssh_session, sshfs_proc.release_channel())},
+      sshfs_process{
+          create_sshfs_process(ssh_session, mp::utils::escape_char(source, '"'), mp::utils::escape_char(target, '"'))},
+      sftp_server_session{make_sftp_session(ssh_session, sshfs_process->release_channel())},
       source_path{source},
+      target_path{target},
       gid_map{gid_map},
       uid_map{uid_map},
       default_uid{default_uid},
       default_gid{default_gid}
 {
 }
 
+mp::SftpServer::~SftpServer()
+{
+    stop_invoked = true;
+}
+
 sftp_attributes_struct mp::SftpServer::attr_from(const QFileInfo& file_info)
 {
     sftp_attributes_struct attr{};
@@ -375,14 +409,50 @@ void mp::SftpServer::run()
         MsgUPtr client_msg{sftp_get_client_message(sftp_server_session.get()), sftp_client_message_free};
         auto msg = client_msg.get();
         if (msg == nullptr)
-            break;
+        {
+            if (stop_invoked)
+                break;
+
+            int status{0};
+            try
+            {
+                status = sshfs_process->exit_code(250ms);
+            }
+            catch (const mp::ExitlessSSHProcessException& e)
+            {
+                status = 1;
+            }
+
+            if (status != 0)
+            {
+                mpl::log(mpl::Level::error, category,
+                         "sshfs in the instance appears to have exited unexpectedly.  Trying to recover.");
+                auto proc = ssh_session.exec(fmt::format("findmnt --source :{}  -o TARGET -n", source_path));
+                auto mount_path = proc.read_std_output();
+                if (!mount_path.empty())
+                {
+                    ssh_session.exec(fmt::format("sudo umount {}", mount_path));
+                }
+
+                sshfs_process = create_sshfs_process(ssh_session, mp::utils::escape_char(source_path, '"'),
+                                                     mp::utils::escape_char(target_path, '"'));
+                sftp_server_session = make_sftp_session(ssh_session, sshfs_process->release_channel());
+
+                continue;
+            }
+            else
+            {
+                break;
+            }
+        }
 
         process_message(msg);
     }
 }
 
 void mp::SftpServer::stop()
 {
+    stop_invoked = true;
     ssh_session.force_shutdown();
 }
 

diff --git a/src/sshfs_mount/sshfs_mount.cpp b/src/sshfs_mount/sshfs_mount.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017-2018 Canonical, Ltd.
+ * Copyright (C) 2017-2019 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
@@ -46,19 +46,6 @@ auto run_cmd(mp::SSHSession& session, std::string&& cmd)
     return run_cmd(session, std::forward<std::string>(cmd), error_handler);
 }
 
-
9E7A
void check_sshfs_is_running(mp::SSHSession& session, mp::SSHProcess& sshfs_process, const std::string& source,
-                            const std::string& target)
-{
-    using namespace std::literals::chrono_literals;
-
-    // Make sure sshfs actually runs
-    std::this_thread::sleep_for(250ms);
-    auto error_handler = [&sshfs_process](mp::SSHProcess&) {
-        throw std::runtime_error(sshfs_process.read_std_error());
-    };
-    run_cmd(session, fmt::format("pgrep -fx \"sshfs.*{}.*{}\"", source, target), error_handler);
-}
-
 void check_sshfs_exists(mp::SSHSession& session)
 {
     auto error_handler = [](mp::SSHProcess& proc) {
@@ -85,28 +72,15 @@ void set_owner_for(mp::SSHSession& session, const std::string& target)
     run_cmd(session, fmt::format("sudo chown {}:{} \"{}\"", vm_user, vm_group, target));
 }
 
-auto create_sshfs_process(mp::SSHSession& session, const std::string& source, const std::string& target)
-{
-    check_sshfs_exists(session);
-    make_target_dir(session, target);
-    set_owner_for(session, target);
-
-    auto sshfs_process = session.exec(fmt::format(
-        "sudo sshfs -o slave -o nonempty -o transform_symlinks -o allow_other :\"{}\" \"{}\"", source, target));
-
-    check_sshfs_is_running(session, sshfs_process, source, target);
-
-    return sshfs_process;
-}
-
 auto make_sftp_server(mp::SSHSession&& session, const std::string& source, const std::string& target,
                       const std::unordered_map<int, int>& gid_map, const std::unordered_map<int, int>& uid_map)
 {
     mpl::log(mpl::Level::debug, category,
              fmt::format("{}:{} {}(source = {}, target = {}, …): ", __FILE__, __LINE__, __FUNCTION__, source, target));
 
-    auto sshfs_proc =
-        create_sshfs_process(session, mp::utils::escape_char(source, '"'), mp::utils::escape_char(target, '"'));
+    check_sshfs_exists(session);
+    make_target_dir(session, target);
+    set_owner_for(session, target);
 
     auto output = run_cmd(session, "id -u");
     mpl::log(mpl::Level::debug, category,
@@ -117,8 +91,8 @@ auto make_sftp_server(mp::SSHSession&& session, const std::string& source, const
              fmt::format("{}:{} {}(): `id -g` = {}", __FILE__, __LINE__, __FUNCTION__, output));
     auto default_gid = std::stoi(output);
 
-    return std::make_unique<mp::SftpServer>(std::move(session), std::move(sshfs_proc), source, gid_map, uid_map,
-                                            default_uid, default_gid);
+    return std::make_unique<mp::SftpServer>(std::move(session), source, target, gid_map, uid_map, default_uid,
+                                            default_gid);
 }
 
 } // namespace anonymous

diff --git a/tests/sftp_server_test_fixture.h b/tests/sftp_server_test_fixture.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018 Canonical, Ltd.
+ * Copyright (C) 2018-2019 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
@@ -28,6 +28,45 @@ namespace multipass
 {
 namespace test
 {
+class ExitStatusMock
+{
+public:
+    ExitStatusMock()
+    {
+        add_channel_cbs = [this](ssh_channel, ssh_channel_callbacks cb) {
+            channel_cbs = cb;
+            return SSH_OK;
+        };
+
+        event_do_poll = [this](auto...) {
+            if (channel_cbs == nullptr)
+                return SSH_ERROR;
+            channel_cbs->channel_exit_status_function(nullptr, nullptr, exit_code, channel_cbs->userdata);
+            return SSH_OK;
+        };
+    }
+
+    ~ExitStatusMock()
+    {
+        add_channel_cbs = std::move(old_add_channel_cbs);
+        event_do_poll = std::move(old_event_do_poll);
+    }
+
+    void return_exit_code(int code)
+    {
+        exit_code = code;
+    }
+
+private:
+    decltype(mock_ssh_add_channel_callbacks)& add_channel_cbs{mock_ssh_add_channel_callbacks};
+    decltype(mock_ssh_add_channel_callbacks) old_add_channel_cbs{std::move(mock_ssh_add_channel_callbacks)};
+    decltype(mock_ssh_event_dopoll)& event_do_poll{mock_ssh_event_dopoll};
+    decltype(mock_ssh_event_dopoll) old_event_do_poll{std::move(mock_ssh_event_dopoll)};
+
+    int exit_code{SSH_OK};
+    ssh_channel_callbacks channel_cbs{nullptr};
+};
+
 struct SftpServerTest : public testing::Test
 {
     SftpServerTest()