From 1bf4c486d34e7091df6b4d1da29665bbb51c1975 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 6 Feb 2024 19:15:15 +0000 Subject: [PATCH 1/4] [qemu] Throw dedicated exception on qemu-img issue So that it can be dealt with specifically. --- .../backends/shared/qemu_img_utils/qemu_img_utils.cpp | 8 ++++---- .../backends/shared/qemu_img_utils/qemu_img_utils.h | 6 ++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.cpp b/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.cpp index 9bcc1b06dd..79dc16ac3f 100644 --- a/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.cpp +++ b/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.cpp @@ -41,10 +41,10 @@ auto mp::backend::checked_exec_qemu_img(std::unique_ptr auto process_state = timeout ? process->execute(*timeout) : process->execute(); if (!process_state.completed_successfully()) { - throw std::runtime_error(fmt::format("{}: qemu-img failed ({}) with output:\n{}", - custom_error_prefix, - process_state.failure_message(), - process->read_all_standard_error())); + throw QemuImgException{fmt::format("{}: qemu-img failed ({}) with output:\n{}", + custom_error_prefix, + process_state.failure_message(), + process->read_all_standard_error())}; } return process; diff --git a/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.h b/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.h index 6d7b89e0b9..89cb0dcf4b 100644 --- a/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.h +++ b/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.h @@ -30,6 +30,12 @@ class QemuImgProcessSpec; namespace backend { +class QemuImgException : public std::runtime_error +{ +public: + using std::runtime_error::runtime_error; +}; + Process::UPtr checked_exec_qemu_img(std::unique_ptr spec, const std::string& custom_error_prefix = "Internal error", std::optional timeout = std::nullopt); From bb8eac725765c583f8c976e83d716549241ab218 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 6 Feb 2024 19:17:02 +0000 Subject: [PATCH 2/4] [qemu] Handle problems amending image in ctor Catch and log `qemu-img` failures when attempting to convert an existing VM image to QCOW v2, in the VM's constructor. --- src/platform/backends/qemu/qemu_virtual_machine.cpp | 11 +++++++++-- .../backends/shared/qemu_img_utils/qemu_img_utils.cpp | 3 ++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index 8f307bfed5..c229a8e1e7 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -211,8 +211,15 @@ mp::QemuVirtualMachine::QemuVirtualMachine(const VirtualMachineDescription& desc monitor{&monitor}, mount_args{mount_args_from_json(monitor.retrieve_metadata_for(vm_name))} { - // convert existing VMs to v3 too (doesn't affect images that are already v3) - mp::backend::amend_to_qcow2_v3(desc.image.image_path); // TODO drop in a couple of releases (going in on v1.13) + try + { + // convert existing VMs to v3 too (doesn't affect images that are already v3) + backend::amend_to_qcow2_v3(desc.image.image_path); // TODO drop in a couple of releases (going in on v1.13) + } + catch (const backend::QemuImgException& e) + { + mpl::log(mpl::Level::error, vm_name, e.what()); + } QObject::connect( this, &QemuVirtualMachine::on_delete_memory_snapshot, this, diff --git a/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.cpp b/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.cpp index 79dc16ac3f..9ce5c514e8 100644 --- a/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.cpp +++ b/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.cpp @@ -90,7 +90,8 @@ mp::Path mp::backend::convert_to_qcow_if_necessary(const mp::Path& image_path) void mp::backend::amend_to_qcow2_v3(const mp::Path& image_path) { checked_exec_qemu_img( - std::make_unique(QStringList{"amend", "-o", "compat=1.1", image_path}, image_path)); + std::make_unique(QStringList{"amend", "-o", "compat=1.1", image_path}, image_path), + "Failed to amend image to QCOW2 v3"); } bool mp::backend::instance_image_has_snapshot(const mp::Path& image_path, QString snapshot_tag) From 3a58e3bd1bce3ce7a280578afc323d8faeb98067 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 6 Feb 2024 19:22:41 +0000 Subject: [PATCH 3/4] [qemu] Extract conversion from the constructor --- .../backends/qemu/qemu_virtual_machine.cpp | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index c229a8e1e7..b64a190476 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -194,6 +194,19 @@ auto generate_metadata(const QStringList& platform_args, const QStringList& proc metadata[mount_data_key] = mount_args_to_json(mount_args); return metadata; } + +void convert_to_qcow2_v3_if_necessary(const mp::Path& image_path, const std::string& vm_name) +{ + try + { + // convert existing VMs to v3 too (doesn't affect images that are already v3) + mp::backend::amend_to_qcow2_v3(image_path); + } + catch (const mp::backend::QemuImgException& e) + { + mpl::log(mpl::Level::error, vm_name, e.what()); + } +} } // namespace mp::QemuVirtualMachine::QemuVirtualMachine(const VirtualMachineDescription& desc, @@ -211,15 +224,8 @@ mp::QemuVirtualMachine::QemuVirtualMachine(const VirtualMachineDescription& desc monitor{&monitor}, mount_args{mount_args_from_json(monitor.retrieve_metadata_for(vm_name))} { - try - { - // convert existing VMs to v3 too (doesn't affect images that are already v3) - backend::amend_to_qcow2_v3(desc.image.image_path); // TODO drop in a couple of releases (going in on v1.13) - } - catch (const backend::QemuImgException& e) - { - mpl::log(mpl::Level::error, vm_name, e.what()); - } + convert_to_qcow2_v3_if_necessary(desc.image.image_path, + vm_name); // TODO drop in a couple of releases (went in on v1.13) QObject::connect( this, &QemuVirtualMachine::on_delete_memory_snapshot, this, From 2a3a4595fe95d41a31e94d2049aaf999e1a62d61 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 6 Feb 2024 23:27:04 +0000 Subject: [PATCH 4/4] [test] Check handling of failure to amend image --- tests/qemu/test_qemu_backend.cpp | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/qemu/test_qemu_backend.cpp b/tests/qemu/test_qemu_backend.cpp index 9e27039a62..285d2bdd81 100644 --- a/tests/qemu/test_qemu_backend.cpp +++ b/tests/qemu/test_qemu_backend.cpp @@ -19,6 +19,7 @@ #include "tests/common.h" #include "tests/mock_environment_helpers.h" +#include "tests/mock_logger.h" #include "tests/mock_process_factory.h" #include "tests/mock_snapshot.h" #include "tests/mock_status_monitor.h" @@ -680,6 +681,29 @@ TEST_F(QemuBackend, ssh_hostname_timeout_throws_and_sets_unknown_state) EXPECT_EQ(machine.state, mp::VirtualMachine::State::unknown); } +TEST_F(QemuBackend, logsErrorOnFailureToConvertToQcow2V3UponConstruction) +{ + mpt::StubVMStatusMonitor stub_monitor{}; + NiceMock mock_qemu_platform{}; + + process_factory->register_callback([this](mpt::MockProcess* process) { + if (process->program().contains("qemu-img") && process->arguments().contains("compat=1.1")) + { + mp::ProcessState exit_state{}; + exit_state.exit_code = 1; + ON_CALL(*process, execute).WillByDefault(Return(exit_state)); + } + else + return handle_external_process_calls(process); + }); + + auto logger_scope = mpt::MockLogger::inject(); + logger_scope.mock_logger->screen_logs(mpl::Level::error); + logger_scope.mock_logger->expect_log(mpl::Level::error, "Failed to amend image to QCOW2 v3"); + + mp::QemuVirtualMachine machine{default_description, &mock_qemu_platform, stub_monitor, instance_dir.path()}; +} + struct PublicSnapshotMakingQemuVM : public mpt::MockVirtualMachineT { using mpt::MockVirtualMachineT::MockVirtualMachineT;