Merge #1789

1789: [daemon] add MULTIPASS_STORAGE support (Fixes #1215) r=Saviq a=Saviq



Co-authored-by: Michał Sawicz <michal@sawicz.net>
Co-authored-by: Michał Sawicz <michal.sawicz@canonical.com>

include/multipass/process/qemuimg_process_spec.h

@@ -29,7 +29,7 @@ namespace multipass
 class QemuImgProcessSpec : public ProcessSpec
 {
 public:
-    explicit QemuImgProcessSpec(const QStringList& args);
+    explicit QemuImgProcessSpec(const QStringList& args, const QString& source_image, const QString& target_image = {});
 
     QString program() const override;
     QStringList arguments() const override;
@@ -38,6 +38,8 @@ class QemuImgProcessSpec : public ProcessSpec
 
 private:
     const QStringList args;
+    const QString source_image;
+    const QString target_image;
 };
 
 } // namespace multipass

snap/snapcraft.yaml

@@ -46,6 +46,7 @@ apps:
       - network-bind
       - network-control
       - lxd
+      - removable-media
   multipass:
     environment:
       <<: &client-environment

src/daemon/daemon.cpp

@@ -606,7 +606,8 @@ mp::InstanceStatus::Status grpc_instance_status_for(const mp::VirtualMachine::St
 mp::MemorySize get_image_size(const mp::VMImage& image)
 {
     QStringList qemuimg_parameters{{"info", image.image_path}};
-    auto qemuimg_process = mp::platform::make_process(std::make_unique<mp::QemuImgProcessSpec>(qemuimg_parameters));
+    auto qemuimg_process =
+        mp::platform::make_process(std::make_unique<mp::QemuImgProcessSpec>(qemuimg_parameters, image.image_path));
     auto process_state = qemuimg_process->execute();
 
     if (!process_state.completed_successfully())

src/daemon/daemon_config.cpp

@@ -104,10 +104,22 @@ std::unique_ptr<const mp::DaemonConfig> mp::DaemonConfigBuilder::build()
     auto multiplexing_logger = std::make_shared<mpl::MultiplexingLogger>(std::move(logger));
     mpl::set_logger(multiplexing_logger);
 
+    auto storage_path = QString::fromUtf8(qgetenv("MULTIPASS_STORAGE"));
+
     if (cache_directory.isEmpty())
-        cache_directory = MP_STDPATHS.writableLocation(StandardPaths::CacheLocation);
+    {
+        if (!storage_path.isEmpty())
+            cache_directory = mp::utils::make_dir(storage_path, "cache");
+        else
+            cache_directory = MP_STDPATHS.writableLocation(StandardPaths::CacheLocation);
+    }
     if (data_directory.isEmpty())
-        data_directory = MP_STDPATHS.writableLocation(StandardPaths::AppDataLocation);
+    {
+        if (!storage_path.isEmpty())
+            data_directory = mp::utils::make_dir(storage_path, "data");
+        else
+            data_directory = MP_STDPATHS.writableLocation(StandardPaths::AppDataLocation);
+    }
     if (url_downloader == nullptr)
         url_downloader = std::make_unique<URLDownloader>(cache_directory, std::chrono::seconds{10});
     if (factory == nullptr)

src/platform/backends/shared/linux/backend_utils.cpp

@@ -133,7 +133,8 @@ void mp::backend::resize_instance_image(const MemorySize& disk_space, const mp::
 {
     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));
+    auto qemuimg_process =
+        mp::platform::make_process(std::make_unique<mp::QemuImgProcessSpec>(qemuimg_parameters, "", image_path));
 
     auto process_state = qemuimg_process->execute(mp::backend::image_resize_timeout);
     if (!process_state.completed_successfully())
@@ -150,7 +151,8 @@ 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});
+    auto qemuimg_info_spec =
+        std::make_unique<mp::QemuImgProcessSpec>(QStringList{"info", "--output=json", image_path}, image_path);
     auto qemuimg_info_process = MP_PROCFACTORY.create_process(std::move(qemuimg_info_spec));
 
     auto process_state = qemuimg_info_process->execute();
@@ -167,7 +169,7 @@ mp::Path mp::backend::convert_to_qcow_if_necessary(const mp::Path& image_path)
     if (image_record["format"].toString() == "raw")
     {
         auto qemuimg_convert_spec = std::make_unique<mp::QemuImgProcessSpec>(
-            QStringList{"convert", "-p", "-O", "qcow2", image_path, qcow2_path});
+            QStringList{"convert", "-p", "-O", "qcow2", image_path, qcow2_path}, image_path, qcow2_path);
         auto qemuimg_convert_process = MP_PROCFACTORY.create_process(std::move(qemuimg_convert_spec));
         process_state = qemuimg_convert_process->execute(mp::backend::image_resize_timeout);
 

src/process/qemuimg_process_spec.cpp

@@ -22,7 +22,9 @@
 namespace mp = multipass;
 namespace mu = multipass::utils;
 
-mp::QemuImgProcessSpec::QemuImgProcessSpec(const QStringList& args) : args{args}
+mp::QemuImgProcessSpec::QemuImgProcessSpec(const QStringList& args, const QString& source_image,
+                                           const QString& target_image)
+    : args{args}, source_image{source_image}, target_image{target_image}
 {
 }
 
@@ -52,8 +54,8 @@ profile %1 flags=(attach_disconnected) {
   # CLASSIC ONLY: need to specify required libs from core snap
   /{,var/lib/snapd/}snap/core18/*/{,usr/}lib/@{multiarch}/{,**/}*.so* rm,
 
-  # Subdirectory containing disk image(s)
-  %5/** rwk,
+  # Images
+%5
 
   # Allow multipassd send qemu-img signals
   signal (receive) peer=%6,
@@ -62,14 +64,13 @@ profile %1 flags=(attach_disconnected) {
 
     /* Customisations depending on if running inside snap or not */
     QString root_dir; // root directory: either "" or $SNAP
-    QString image_dir;
+    QString images;
     QString extra_capabilities;
     QString signal_peer; // who can send kill signal to qemu-img
 
     try
     {
         root_dir = mu::snap_dir();
-        image_dir = mu::snap_common_dir();         // FIXME - am guessing we work inside this directory
         signal_peer = "snap.multipass.multipassd"; // only multipassd can send qemu-img signals
     }
     catch (mp::SnapEnvironmentException&)
@@ -78,9 +79,13 @@ profile %1 flags=(attach_disconnected) {
             "capability dac_read_search,\n    capability dac_override,"; // FIXME - unclear why this is required when
                                                                          // not snap confined
         signal_peer = "unconfined";
-        image_dir = ""; // FIXME - Do not know where disk images might be, as passed by argument
     }
 
-    return profile_template.arg(apparmor_profile_name(), extra_capabilities, root_dir, program(), image_dir,
-                                signal_peer);
+    if (!source_image.isEmpty())
+        images.append(QString("  %1 rk,\n").arg(source_image));
+
+    if (!target_image.isEmpty())
+        images.append(QString("  %1 rwk,\n").arg(target_image));
+
+    return profile_template.arg(apparmor_profile_name(), extra_capabilities, root_dir, program(), images, signal_peer);
 }

tests/test_daemon.cpp

@@ -446,6 +446,35 @@ TEST_F(Daemon, proxy_contains_valid_info)
     EXPECT_THAT(config->network_proxy->port(), port);
 }
 
+TEST_F(Daemon, data_path_valid)
+{
+    QTemporaryDir xdg_data_dir;
+
+    mpt::SetEnvScope data("XDG_DATA_HOME", xdg_data_dir.filePath("data").toUtf8());
+    mpt::SetEnvScope cache("XDG_CACHE_HOME", xdg_data_dir.filePath("cache").toUtf8());
+
+    config_builder.data_directory = "";
+    config_builder.cache_directory = "";
+    auto config = config_builder.build();
+
+    EXPECT_EQ(config->data_directory.toStdString(), xdg_data_dir.filePath("data/multipass_tests").toStdString());
+    EXPECT_EQ(config->cache_directory.toStdString(), xdg_data_dir.filePath("cache/multipass_tests").toStdString());
+}
+
+TEST_F(Daemon, data_path_with_storage_valid)
+{
+    QTemporaryDir storage_dir;
+
+    mpt::SetEnvScope storage("MULTIPASS_STORAGE", storage_dir.path().toUtf8());
+
+    config_builder.data_directory = "";
+    config_builder.cache_directory = "";
+    auto config = config_builder.build();
+
+    EXPECT_EQ(config->data_directory.toStdString(), storage_dir.filePath("data").toStdString());
+    EXPECT_EQ(config->cache_directory.toStdString(), storage_dir.filePath("cache").toStdString());
+}
+
 namespace
 {
 struct DaemonCreateLaunchTestSuite : public Daemon, public WithParamInterface<std::string>

tests/test_qemuimg_process_spec.cpp

@@ -29,71 +29,100 @@ using namespace testing;
 
 TEST(TestQemuImgProcessSpec, program_correct)
 {
-    mp::QemuImgProcessSpec spec({});
+    mp::QemuImgProcessSpec spec({}, "");
 
     EXPECT_EQ(spec.program(), "qemu-img");
 }
 
 TEST(TestQemuImgProcessSpec, default_arguments_correct)
 {
-    mp::QemuImgProcessSpec spec({});
+    mp::QemuImgProcessSpec spec({}, "");
 
     EXPECT_EQ(spec.arguments(), QStringList());
 }
 
 TEST(TestQemuImgProcessSpec, arguments_set_correctly)
 {
     QStringList args{"-one", "--two"};
-    mp::QemuImgProcessSpec spec(args);
+    mp::QemuImgProcessSpec spec(args, "");
 
     EXPECT_EQ(spec.arguments(), args);
 }
 
 TEST(TestQemuImgProcessSpec, apparmor_profile_has_correct_name)
 {
-    mp::QemuImgProcessSpec spec({});
+    mp::QemuImgProcessSpec spec({}, "");
 
     EXPECT_TRUE(spec.apparmor_profile().contains("profile multipass.qemu-img"));
 }
 
 TEST(TestQemuImgProcessSpec, no_apparmor_profile_identifier)
 {
-    mp::QemuImgProcessSpec spec({});
+    mp::QemuImgProcessSpec spec({}, "");
 
     EXPECT_EQ(spec.identifier(), "");
 }
 
 TEST(TestQemuImgProcessSpec, apparmor_profile_running_as_snap_correct)
 {
     const QByteArray snap_name{"multipass"};
-    QTemporaryDir snap_dir, common_dir;
+    QTemporaryDir snap_dir;
+    QString source_image{"/source/image/file"};
 
     mpt::SetEnvScope e("SNAP", snap_dir.path().toUtf8());
-    mpt::SetEnvScope e2("SNAP_COMMON", common_dir.path().toUtf8());
-    mpt::SetEnvScope e3("SNAP_NAME", snap_name);
-    mp::QemuImgProcessSpec spec({});
+    mpt::SetEnvScope e2("SNAP_NAME", snap_name);
+    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)));
+}
+
+TEST(TestQemuImgProcessSpec, apparmor_profile_running_as_snap_with_target_correct)
+{
+    const QByteArray snap_name{"multipass"};
+    QTemporaryDir snap_dir;
+    QString source_image{"/source/image/file"}, target_image{"/target/image/file"};
+
+    mpt::SetEnvScope e("SNAP", snap_dir.path().toUtf8());
+    mpt::SetEnvScope e2("SNAP_NAME", snap_name);
+    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(target_image)));
+}
+
+TEST(TestQemuImgProcessSpec, apparmor_profile_running_as_snap_with_only_target_correct)
+{
+    const QByteArray snap_name{"multipass"};
+    QTemporaryDir snap_dir;
+    QString target_image{"/target/image/file"};
+
+    mpt::SetEnvScope e("SNAP", snap_dir.path().toUtf8());
+    mpt::SetEnvScope e2("SNAP_NAME", snap_name);
+    mp::QemuImgProcessSpec spec({}, "", 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/** rwk,").arg(common_dir.path())));
+    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1 rwk,").arg(target_image)));
 }
 
 TEST(TestQemuImgProcessSpec, apparmor_profile_running_as_symlinked_snap_correct)
 {
     const QByteArray snap_name{"multipass"};
     QTemporaryDir snap_dir, snap_link_dir, common_dir, common_link_dir;
+    QString source_image{"/source/image/file"};
 
     snap_link_dir.remove();
     common_link_dir.remove();
     QFile::link(snap_dir.path(), snap_link_dir.path());
     QFile::link(common_dir.path(), common_link_dir.path());
 
     mpt::SetEnvScope e("SNAP", snap_link_dir.path().toUtf8());
-    mpt::SetEnvScope e2("SNAP_COMMON", common_link_dir.path().toUtf8());
     mpt::SetEnvScope e3("SNAP_NAME", snap_name);
-    mp::QemuImgProcessSpec spec({});
+    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/** rwk,").arg(common_dir.path())));
+    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1 rk,").arg(source_image)));
 }
 
 TEST(TestQemuImgProcessSpec, apparmor_profile_not_running_as_snap_correct)
@@ -102,7 +131,7 @@ TEST(TestQemuImgProcessSpec, apparmor_profile_not_running_as_snap_correct)
 
     mpt::UnsetEnvScope e("SNAP");
     mpt::SetEnvScope e2("SNAP_NAME", snap_name);
-    mp::QemuImgProcessSpec spec({});
+    mp::QemuImgProcessSpec spec({}, "");
 
     EXPECT_TRUE(spec.apparmor_profile().contains("capability dac_read_search,"));
     EXPECT_TRUE(spec.apparmor_profile().contains(" /usr/bin/qemu-img ixr,")); // space wanted