From 55d5eed556ceca961ecf6e179cfc70fd99bbdd3c Mon Sep 17 00:00:00 2001 From: Chris Townsend Date: Wed, 7 Feb 2024 15:05:19 +0000 Subject: [PATCH] Merge pull request #3400 from canonical/handle-amend-errors Handle failure to convert to QCOW2 v3 --- .../backends/qemu/qemu_virtual_machine.cpp | 17 +++++++++++-- .../shared/qemu_img_utils/qemu_img_utils.cpp | 11 +++++---- .../shared/qemu_img_utils/qemu_img_utils.h | 6 +++++ tests/qemu/test_qemu_backend.cpp | 24 +++++++++++++++++++ 4 files changed, 51 insertions(+), 7 deletions(-) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index b34e94918d..36fb901a31 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,8 +224,8 @@ 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) + 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, 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..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 @@ -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; @@ -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) 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); diff --git a/tests/qemu/test_qemu_backend.cpp b/tests/qemu/test_qemu_backend.cpp index bc582ed9d2..9a23aba18c 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_status_monitor.h" #include "tests/stub_process_factory.h" @@ -675,6 +676,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()}; +} + TEST_F(QemuBackend, lists_no_networks) { EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) {