Merge pull request #3684 from canonical/fix-stop-failed-no-error

Fix `stop` succeeding when instance isn't stopped
canonical · Sep 30, 2024 · 04a3a73 · 04a3a73
1 parent 86053b9
commit 04a3a73
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 10 deletions.
diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp
@@ -29,6 +29,7 @@
 #include <multipass/logging/log.h>
 #include <multipass/memory_size.h>
 #include <multipass/platform.h>
+#include <multipass/top_catch_all.h>
 #include <multipass/utils.h>
 #include <multipass/vm_mount.h>
 #include <multipass/vm_status_monitor.h>
@@ -286,14 +287,16 @@ mp::QemuVirtualMachine::~QemuVirtualMachine()
     {
         update_shutdown_status = false;
 
-        if (state == State::running)
-        {
-            suspend();
-        }
-        else
-        {
-            shutdown();
-        }
+        mp::top_catch_all(vm_name, [this]() {
+            if (state == State::running)
+            {
+                suspend();
+            }
+            else
+            {
+                shutdown();
+            }
+        });
     }
 }
 
@@ -372,7 +375,7 @@ void mp::QemuVirtualMachine::shutdown(ShutdownPolicy shutdown_policy)
             if (vm_process != nullptr && !vm_process->wait_for_finished(kill_process_timeout))
             {
                 throw std::runtime_error{
-                    fmt::format("The QEMU process did not finish within {} seconds after being killed",
+                    fmt::format("The QEMU process did not finish within {} milliseconds after being killed",
                                 kill_process_timeout)};
             }
         }
@@ -404,7 +407,17 @@ void mp::QemuVirtualMachine::shutdown(ShutdownPolicy shutdown_policy)
         if (vm_process && vm_process->running())
         {
             vm_process->write(qmp_execute_json("system_powerdown"));
-            vm_process->wait_for_finished(shutdown_timeout);
+            if (vm_process->wait_for_finished(shutdown_timeout))
+            {
+                lock.lock();
+                state = State::off;
+            }
+            else
+            {
+                throw std::runtime_error{
+                    fmt::format("The QEMU process did not finish within {} milliseconds after being shutdown",
+                                shutdown_timeout)};
+            }
         }
     }
 }

diff --git a/tests/qemu/test_qemu_backend.cpp b/tests/qemu/test_qemu_backend.cpp
@@ -299,6 +299,44 @@ TEST_F(QemuBackend, throws_when_shutdown_while_starting)
     EXPECT_EQ(machine->current_state(), mp::VirtualMachine::State::off);
 }
 
+TEST_F(QemuBackend, throws_on_shutdown_timeout)
+{
+    static const std::string sub_error_msg1{"The QEMU process did not finish within "};
+    static const std::string sub_error_msg2{"seconds after being shutdown"};
+
+    mpt::MockProcess* vmproc = nullptr;
+    process_factory->register_callback([&vmproc](mpt::MockProcess* process) {
+        if (process->program().startsWith("qemu-system-") &&
+            !process->arguments().contains("-dump-vmstate")) // we only care about the actual vm process
+        {
+            vmproc = process; // save this to control later
+        }
+    });
+
+    EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) {
+        return std::move(mock_qemu_platform);
+    });
+
+    mpt::StubVMStatusMonitor stub_monitor;
+    mp::QemuVirtualMachineFactory backend{data_dir.path()};
+
+    auto machine = backend.create_virtual_machine(default_description, key_provider, stub_monitor);
+
+    machine->start();
+
+    ASSERT_TRUE(vmproc);
+    EXPECT_CALL(*vmproc, wait_for_finished).WillOnce(Return(false)).WillRepeatedly(Return(true));
+    EXPECT_CALL(*vmproc, running).WillOnce(Return(true)).WillRepeatedly(Return(false));
+
+    machine->state = mp::VirtualMachine::State::running;
+
+    MP_EXPECT_THROW_THAT(machine->shutdown(),
+                         std::runtime_error,
+                         mpt::match_what(AllOf(HasSubstr(sub_error_msg1), HasSubstr(sub_error_msg2))));
+
+    EXPECT_NE(machine->current_state(), mp::VirtualMachine::State::off);
+}
+
 TEST_F(QemuBackend, includes_error_when_shutdown_while_starting)
 {
     constexpr auto error_msg = "failing spectacularly";