Merge pull request #3230 from canonical/resize-with-snapshots

[snapshots] Enable resizing QEMU instances with snapshots

src/platform/backends/qemu/qemu_snapshot.cpp

@@ -29,7 +29,6 @@
 #include <memory>
 
 namespace mp = multipass;
-namespace mpp = mp::platform;
 
 namespace
 {
@@ -50,18 +49,6 @@ std::unique_ptr<mp::QemuImgProcessSpec> make_delete_spec(const QString& tag, con
     return std::make_unique<mp::QemuImgProcessSpec>(QStringList{"snapshot", "-d", tag, image_path},
                                                     /* src_img = */ "", image_path);
 }
-
-void checked_exec_qemu_img(std::unique_ptr<mp::QemuImgProcessSpec> spec)
-{
-    auto process = mpp::make_process(std::move(spec));
-
-    auto process_state = process->execute();
-    if (!process_state.completed_successfully())
-    {
-        throw std::runtime_error(fmt::format("Internal error: qemu-img failed ({}) with output:\n{}",
-                                             process_state.failure_message(), process->read_all_standard_error()));
-    }
-}
 } // namespace
 
 mp::QemuSnapshot::QemuSnapshot(const std::string& name, const std::string& comment, const VMSpecs& specs,
@@ -85,12 +72,12 @@ void mp::QemuSnapshot::capture_impl()
         throw std::runtime_error{fmt::format(
             "A snapshot with the same tag already exists in the image. Image: {}; tag: {})", image_path, tag)};
 
-    checked_exec_qemu_img(make_capture_spec(tag, image_path));
+    mp::backend::checked_exec_qemu_img(make_capture_spec(tag, image_path));
 }
 
 void mp::QemuSnapshot::erase_impl()
 {
-    checked_exec_qemu_img(make_delete_spec(derive_id(), image_path));
+    mp::backend::checked_exec_qemu_img(make_delete_spec(derive_id(), image_path));
 }
 
 void mp::QemuSnapshot::apply_impl()
@@ -102,6 +89,6 @@ void mp::QemuSnapshot::apply_impl()
     desc.mem_size = get_mem_size();
     desc.disk_space = get_disk_space();
 
-    checked_exec_qemu_img(make_restore_spec(derive_id(), image_path));
+    mp::backend::checked_exec_qemu_img(make_restore_spec(derive_id(), image_path));
     rollback.dismiss();
 }

src/platform/backends/qemu/qemu_virtual_machine.cpp

@@ -208,6 +208,9 @@ 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)
+
     QObject::connect(
         this, &QemuVirtualMachine::on_delete_memory_snapshot, this,
         [this] {

src/platform/backends/qemu/qemu_virtual_machine_factory.cpp

@@ -56,6 +56,7 @@ mp::VMImage mp::QemuVirtualMachineFactory::prepare_source_image(const mp::VMImag
 {
     VMImage image{source_image};
     image.image_path = mp::backend::convert_to_qcow_if_necessary(source_image.image_path);
+    mp::backend::amend_to_qcow2_v3(image.image_path);
     return image;
 }
 

src/platform/backends/shared/base_virtual_machine.cpp

@@ -520,8 +520,7 @@ void BaseVirtualMachine::restore_snapshot(const QDir& snapshot_dir, const std::s
 
     auto snapshot = get_snapshot(name);
 
-    // TODO@snapshots convert into runtime_errors (persisted info could have been tampered with)
-    assert(specs.disk_space == snapshot->get_disk_space() && "resizing VMs with snapshots isn't yet supported");
+    // TODO@snapshots convert into runtime_error (persisted info could have been tampered with)
     assert(snapshot->get_state() == St::off || snapshot->get_state() == St::stopped);
 
     snapshot->apply();
@@ -532,6 +531,7 @@ void BaseVirtualMachine::restore_snapshot(const QDir& snapshot_dir, const std::s
     specs.state = snapshot->get_state();
     specs.num_cores = snapshot->get_num_cores();
     specs.mem_size = snapshot->get_mem_size();
+    specs.disk_space = snapshot->get_disk_space();
     specs.mounts = snapshot->get_mounts();
     specs.metadata = snapshot->get_metadata();
 

src/platform/backends/shared/qemu_img_utils/qemu_img_utils.cpp

@@ -30,21 +30,31 @@
 #include <QStringList>
 
 namespace mp = multipass;
+namespace mpp = mp::platform;
 
-void mp::backend::resize_instance_image(const MemorySize& disk_space, const mp::Path& image_path)
+auto mp::backend::checked_exec_qemu_img(std::unique_ptr<mp::QemuImgProcessSpec> spec,
+                                        const std::string& custom_error_prefix, std::optional<int> timeout)
+    -> Process::UPtr
 {
-    auto disk_size = QString::number(disk_space.in_bytes()); // format documented in `man qemu-img` (look for "size")
-    QStringList qemuimg_parameters{{"resize", image_path, disk_size}};
-    auto qemuimg_process =
-        mp::platform::make_process(std::make_unique<mp::QemuImgProcessSpec>(qemuimg_parameters, "", image_path));
+    auto process = mpp::make_process(std::move(spec));
 
-    auto process_state = qemuimg_process->execute(mp::image_resize_timeout);
+    auto process_state = timeout ? process->execute(*timeout) : process->execute();
     if (!process_state.completed_successfully())
     {
-        throw std::runtime_error(fmt::format("Cannot resize instance image: qemu-img failed ({}) with output:\n{}",
-                                             process_state.failure_message(),
-                                             qemuimg_process->read_all_standard_error()));
+        throw std::runtime_error(fmt::format("{}: qemu-img failed ({}) with output:\n{}", custom_error_prefix,
+                                             process_state.failure_message(), process->read_all_standard_error()));
     }
+
+    return process;
+}
+
+void mp::backend::resize_instance_image(const MemorySize& disk_space, const mp::Path& image_path)
+{
+    auto disk_size = QString::number(disk_space.in_bytes()); // format documented in `man qemu-img` (look for "size")
+    QStringList qemuimg_parameters{{"resize", image_path, disk_size}};
+
+    checked_exec_qemu_img(std::make_unique<mp::QemuImgProcessSpec>(qemuimg_parameters, "", image_path),
+                          "Cannot resize instance image", mp::image_resize_timeout);
 }
 
 mp::Path mp::backend::convert_to_qcow_if_necessary(const mp::Path& image_path)
@@ -53,17 +63,9 @@ mp::Path mp::backend::convert_to_qcow_if_necessary(const mp::Path& image_path)
     // TODO: we could support converting from other the image formats that qemu-img can deal with
     const auto qcow2_path{image_path + ".qcow2"};
 
-    auto qemuimg_info_spec =
-        std::make_unique<mp::QemuImgProcessSpec>(QStringList{"info", "--output=json", image_path}, image_path);
-    auto qemuimg_info_process = mp::platform::make_process(std::move(qemuimg_info_spec));
-
-    auto process_state = qemuimg_info_process->execute();
-    if (!process_state.completed_successfully())
-    {
-        throw std::runtime_error(fmt::format("Cannot read image format: qemu-img failed ({}) with output:\n{}",
-                                             process_state.failure_message(),
-                                             qemuimg_info_process->read_all_standard_error()));
-    }
+    auto qemuimg_info_process = checked_exec_qemu_img(
+        std::make_unique<mp::QemuImgProcessSpec>(QStringList{"info", "--output=json", image_path}, image_path),
+        "Cannot read image format");
 
     auto image_info = qemuimg_info_process->read_all_standard_output();
     auto image_record = QJsonDocument::fromJson(QString(image_info).toUtf8(), nullptr).object();
@@ -72,15 +74,8 @@ mp::Path mp::backend::convert_to_qcow_if_necessary(const mp::Path& image_path)
     {
         auto qemuimg_convert_spec = std::make_unique<mp::QemuImgProcessSpec>(
             QStringList{"convert", "-p", "-O", "qcow2", image_path, qcow2_path}, image_path, qcow2_path);
-        auto qemuimg_convert_process = mp::platform::make_process(std::move(qemuimg_convert_spec));
-        process_state = qemuimg_convert_process->execute(mp::image_resize_timeout);
-
-        if (!process_state.completed_successfully())
-        {
-            throw std::runtime_error(
-                fmt::format("Failed to convert image format: qemu-img failed ({}) with output:\n{}",
-                            process_state.failure_message(), qemuimg_convert_process->read_all_standard_error()));
-        }
+        auto qemuimg_convert_process =
+            checked_exec_qemu_img(std::move(qemuimg_convert_spec), "Failed to convert image format");
         return qcow2_path;
     }
     else
@@ -89,18 +84,17 @@ mp::Path mp::backend::convert_to_qcow_if_necessary(const mp::Path& image_path)
     }
 }
 
+void mp::backend::amend_to_qcow2_v3(const multipass::Path& image_path)
+{
+    checked_exec_qemu_img(
+        std::make_unique<mp::QemuImgProcessSpec>(QStringList{"amend", "-o", "compat=1.1", image_path}, image_path));
+}
+
 bool mp::backend::instance_image_has_snapshot(const mp::Path& image_path, QString snapshot_tag)
 {
-    auto process = mp::platform::make_process(
+    auto process = checked_exec_qemu_img(
         std::make_unique<mp::QemuImgProcessSpec>(QStringList{"snapshot", "-l", image_path}, image_path));
 
-    auto process_state = process->execute();
-    if (!process_state.completed_successfully())
-    {
-        throw std::runtime_error(fmt::format("Internal error: qemu-img failed ({}) with output:\n{}",
-                                             process_state.failure_message(), process->read_all_standard_error()));
-    }
-
     QRegularExpression regex{snapshot_tag.append(R"(\s)")};
     return QString{process->read_all_standard_output()}.contains(regex);
 }

src/platform/backends/shared/qemu_img_utils/qemu_img_utils.h

@@ -19,15 +19,23 @@
 #define MULTIPASS_QEMU_IMG_UTILS_H
 
 #include <multipass/path.h>
+#include <multipass/platform.h>
+
+#include <optional>
 
 namespace multipass
 {
 class MemorySize;
+class QemuImgProcessSpec;
 
 namespace backend
 {
+Process::UPtr checked_exec_qemu_img(std::unique_ptr<QemuImgProcessSpec> spec,
+                                    const std::string& custom_error_prefix = "Internal error",
+                                    std::optional<int> timeout = std::nullopt);
 void resize_instance_image(const MemorySize& disk_space, const multipass::Path& image_path);
 Path convert_to_qcow_if_necessary(const Path& image_path);
+void amend_to_qcow2_v3(const Path& image_path);
 bool instance_image_has_snapshot(const Path& image_path, QString snapshot_tag);
 
 } // namespace backend

src/process/qemuimg_process_spec.cpp

@@ -83,7 +83,7 @@ profile %1 flags=(attach_disconnected) {
     }
 
     if (!source_image.isEmpty())
-        images.append(QString("  %1 rk,\n").arg(source_image));
+        images.append(QString("  %1 rwk,\n").arg(source_image)); // allow amending to qcow2 v3
 
     if (!target_image.isEmpty())
         images.append(QString("  %1 rwk,\n").arg(target_image));

tests/test_qemuimg_process_spec.cpp

@@ -75,7 +75,7 @@ TEST(TestQemuImgProcessSpec, apparmor_profile_running_as_snap_correct)
     mp::QemuImgProcessSpec spec({}, source_image);
 
     EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/usr/bin/qemu-img ixr,").arg(snap_dir.path())));
-    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1 rk,").arg(source_image)));
+    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1 rwk,").arg(source_image)));
 }
 
 TEST(TestQemuImgProcessSpec, apparmor_profile_running_as_snap_with_target_correct)
@@ -89,7 +89,7 @@ TEST(TestQemuImgProcessSpec, apparmor_profile_running_as_snap_with_target_correc
     mp::QemuImgProcessSpec spec({}, source_image, target_image);
 
     EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/usr/bin/qemu-img ixr,").arg(snap_dir.path())));
-    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1 rk,").arg(source_image)));
+    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1 rwk,").arg(source_image)));
     EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1 rwk,").arg(target_image)));
 }
 
@@ -125,7 +125,7 @@ TEST(TestQemuImgProcessSpec,
     mp::QemuImgProcessSpec spec({}, source_image);
 
     EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/usr/bin/qemu-img ixr,").arg(snap_dir.path())));
-    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1 rk,").arg(source_image)));
+    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1 rwk,").arg(source_image)));
 }
 
 TEST(TestQemuImgProcessSpec, apparmor_profile_not_running_as_snap_correct)