From bfd2686842c7066ec855b75ffc5e80621341a929 Mon Sep 17 00:00:00 2001 From: Trevor Shoe Date: Wed, 18 Sep 2024 13:57:03 -0400 Subject: [PATCH 1/5] [qemu] No longer ignore shutdown timeout --- .../backends/qemu/qemu_virtual_machine.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index 1bd5e1680b..3722c0e4ac 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -372,7 +372,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 {} miliseconds after being killed", kill_process_timeout)}; } } @@ -404,7 +404,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 {} miliseconds after being shutdown", + shutdown_timeout)}; + } } } } From 265d141bace683a923ee8db1788d484bf3a40d94 Mon Sep 17 00:00:00 2001 From: Trevor Shoe Date: Wed, 18 Sep 2024 15:24:04 -0400 Subject: [PATCH 2/5] [qemu] Fix typo "miliseconds" -> "milliseconds" --- src/platform/backends/qemu/qemu_virtual_machine.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index 3722c0e4ac..b866de3c02 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -372,7 +372,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 {} miliseconds after being killed", + fmt::format("The QEMU process did not finish within {} milliseconds after being killed", kill_process_timeout)}; } } @@ -412,7 +412,7 @@ void mp::QemuVirtualMachine::shutdown(ShutdownPolicy shutdown_policy) else { throw std::runtime_error{ - fmt::format("The QEMU process did not finish within {} miliseconds after being shutdown", + fmt::format("The QEMU process did not finish within {} milliseconds after being shutdown", shutdown_timeout)}; } } From 4729fe4bbdb9e2bfd8f1b6328d6a8826a6ea9baf Mon Sep 17 00:00:00 2001 From: Trevor Shoe Date: Wed, 18 Sep 2024 16:49:24 -0400 Subject: [PATCH 3/5] [qemu] Handle exceptions gracefully in destructor --- .../backends/qemu/qemu_virtual_machine.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index b866de3c02..67c337bd11 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -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(); + } + }); } } From 61cb8c99b3593bfd131bba60bfd315d46e8a0ae4 Mon Sep 17 00:00:00 2001 From: Trevor Shoe Date: Fri, 20 Sep 2024 11:40:48 -0400 Subject: [PATCH 4/5] [unit test][qemu] Add test for failed shutdown --- tests/qemu/test_qemu_backend.cpp | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/qemu/test_qemu_backend.cpp b/tests/qemu/test_qemu_backend.cpp index 0647d0286c..e3da465bf7 100644 --- a/tests/qemu/test_qemu_backend.cpp +++ b/tests/qemu/test_qemu_backend.cpp @@ -300,6 +300,38 @@ TEST_F(QemuBackend, throws_when_shutdown_while_starting) EXPECT_EQ(machine->current_state(), mp::VirtualMachine::State::off); } +TEST_F(QemuBackend, throws_on_shutdown_timeout) +{ + 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; + + EXPECT_THROW(machine->shutdown(), std::runtime_error); + EXPECT_NE(machine->current_state(), mp::VirtualMachine::State::off); +} + TEST_F(QemuBackend, includes_error_when_shutdown_while_starting) { constexpr auto error_msg = "failing spectacularly"; From dd7cfb0d2fb184f07a7a48d63a226d995c15afe1 Mon Sep 17 00:00:00 2001 From: Trevor Shoe Date: Wed, 25 Sep 2024 09:23:23 -0400 Subject: [PATCH 5/5] [unit test][qemu] Use MP_EXPECT_THROW_THAT to check error message --- tests/qemu/test_qemu_backend.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/qemu/test_qemu_backend.cpp b/tests/qemu/test_qemu_backend.cpp index e3da465bf7..1344012681 100644 --- a/tests/qemu/test_qemu_backend.cpp +++ b/tests/qemu/test_qemu_backend.cpp @@ -302,6 +302,9 @@ TEST_F(QemuBackend, throws_when_shutdown_while_starting) 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-") && @@ -328,7 +331,10 @@ TEST_F(QemuBackend, throws_on_shutdown_timeout) machine->state = mp::VirtualMachine::State::running; - EXPECT_THROW(machine->shutdown(), std::runtime_error); + 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); }