Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 24 additions & 13 deletions src/platform/backends/hyperv/hyperv_snapshot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,19 @@ namespace mpl = mp::logging;

namespace
{
QString quoted(const QString& s)
std::string quoted(const std::string& s)
{
return '"' + s + '"';
}

bool snapshot_exists(mp::PowerShell& ps, const QString& vm_name, const QString& id)
bool snapshot_exists(mp::PowerShell& ps, const QString& vm_name, const std::string& id)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly speaking, should we not move this QString to std::string as well?

{
static const auto expected_error = QStringLiteral("ObjectNotFound");

QString output_err;
if (ps.run({"Get-VMCheckpoint", "-VMName", vm_name, "-Name", id}, nullptr, &output_err))
if (ps.run({"Get-VMCheckpoint", "-VMName", vm_name, "-Name", QString::fromStdString(id)},
nullptr,
&output_err))
return true;

if (!output_err.contains(expected_error))
Expand All @@ -55,7 +57,7 @@ bool snapshot_exists(mp::PowerShell& ps, const QString& vm_name, const QString&
return false; // we're good: the command failed with the expected error
}

void require_unique_id(mp::PowerShell& ps, const QString& vm_name, const QString& id)
void require_unique_id(mp::PowerShell& ps, const QString& vm_name, const std::string& id)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idem

{
if (snapshot_exists(ps, vm_name, id))
throw std::runtime_error{
Expand All @@ -78,7 +80,7 @@ mp::HyperVSnapshot::HyperVSnapshot(const std::string& name,
{
}

mp::HyperVSnapshot::HyperVSnapshot(const QString& filename,
mp::HyperVSnapshot::HyperVSnapshot(const std::filesystem::path& filename,
HyperVVirtualMachine& vm,
const VirtualMachineDescription& desc,
PowerShell& power_shell)
Expand All @@ -92,16 +94,21 @@ mp::HyperVSnapshot::HyperVSnapshot(const QString& filename,
void mp::HyperVSnapshot::capture_impl()
{
require_unique_id(power_shell, vm_name, quoted_id);
power_shell.easy_run({"Checkpoint-VM", "-Name", vm_name, "-SnapshotName", quoted_id},
"Could not create snapshot");
power_shell.easy_run(
{"Checkpoint-VM", "-Name", vm_name, "-SnapshotName", QString::fromStdString(quoted_id)},
"Could not create snapshot");
}

void mp::HyperVSnapshot::erase_impl()
{
if (snapshot_exists(power_shell, vm_name, quoted_id))
power_shell.easy_run(
{"Remove-VMCheckpoint", "-VMName", vm_name, "-Name", quoted_id, "-Confirm:$false"},
"Could not delete snapshot");
power_shell.easy_run({"Remove-VMCheckpoint",
"-VMName",
vm_name,
"-Name",
QString::fromStdString(quoted_id),
"-Confirm:$false"},
"Could not delete snapshot");
else
mpl::warn(vm_name.toStdString(),
"Could not find underlying Hyper-V snapshot for \"{}\". Ignoring...",
Expand All @@ -110,7 +117,11 @@ void mp::HyperVSnapshot::erase_impl()

void mp::HyperVSnapshot::apply_impl()
{
power_shell.easy_run(
{"Restore-VMCheckpoint", "-VMName", vm_name, "-Name", quoted_id, "-Confirm:$false"},
"Could not apply snapshot");
power_shell.easy_run({"Restore-VMCheckpoint",
"-VMName",
vm_name,
"-Name",
QString::fromStdString(quoted_id),
"-Confirm:$false"},
"Could not apply snapshot");
}
4 changes: 2 additions & 2 deletions src/platform/backends/hyperv/hyperv_snapshot.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class HyperVSnapshot : public BaseSnapshot
const QString& vm_name,
HyperVVirtualMachine& vm,
PowerShell& power_shell);
HyperVSnapshot(const QString& filename,
HyperVSnapshot(const std::filesystem::path& filename,
HyperVVirtualMachine& vm,
const VirtualMachineDescription& desc,
PowerShell& power_shell);
Expand All @@ -48,7 +48,7 @@ class HyperVSnapshot : public BaseSnapshot
void apply_impl() override;

private:
const QString quoted_id;
const std::string quoted_id;
const QString vm_name;
PowerShell& power_shell;
};
Expand Down
5 changes: 4 additions & 1 deletion src/platform/backends/hyperv/hyperv_virtual_machine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -548,5 +548,8 @@ auto mp::HyperVVirtualMachine::make_specific_snapshot(const std::string& snapsho
auto mp::HyperVVirtualMachine::make_specific_snapshot(const QString& filename)
-> std::shared_ptr<Snapshot>
{
return std::make_shared<HyperVSnapshot>(filename, *this, desc, *power_shell);
return std::make_shared<HyperVSnapshot>(MP_PLATFORM.qstr_to_path(filename),
*this,
desc,
*power_shell);
}
9 changes: 5 additions & 4 deletions src/platform/backends/qemu/qemu_img_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
#include <multipass/platform.h>
#include <multipass/process/qemuimg_process_spec.h>

#include <QRegularExpression>
#include <regex>

#include <QString>
#include <QStringList>

Expand Down Expand Up @@ -146,10 +147,10 @@ std::filesystem::path mp::backend::convert_to_raw(const std::filesystem::path& i
}

bool mp::backend::instance_image_has_snapshot(const std::filesystem::path& image_path,
QString snapshot_tag)
const std::string& snapshot_tag)
{
QRegularExpression regex{snapshot_tag.append(R"(\s)")};
return QString{snapshot_list_output(image_path)}.contains(regex);
std::regex regex{snapshot_tag + R"(\s)"};
return std::regex_search(snapshot_list_output(image_path).toStdString(), regex);
}

QByteArray mp::backend::snapshot_list_output(const std::filesystem::path& image_path)
Expand Down
4 changes: 3 additions & 1 deletion src/platform/backends/qemu/qemu_img_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include <filesystem>
#include <optional>
#include <string>

namespace multipass
{
Expand All @@ -44,7 +45,8 @@ void resize_instance_image(const MemorySize& disk_space, const std::filesystem::
std::filesystem::path convert_to_qcow_if_necessary(const std::filesystem::path& image_path);
void amend_to_qcow2_v3(const std::filesystem::path& image_path);
std::filesystem::path convert_to_raw(const std::filesystem::path& image_path);
bool instance_image_has_snapshot(const std::filesystem::path& image_path, QString snapshot_tag);
bool instance_image_has_snapshot(const std::filesystem::path& image_path,
const std::string& snapshot_tag);
QByteArray snapshot_list_output(const std::filesystem::path& image_path);
void delete_snapshot_from_image(const std::filesystem::path& image_path,
const QString& snapshot_tag);
Expand Down
23 changes: 16 additions & 7 deletions src/platform/backends/qemu/qemu_snapshot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,29 +32,38 @@ namespace mp = multipass;

namespace
{
std::unique_ptr<mp::QemuImgProcessSpec> make_capture_spec(const QString& tag,
std::unique_ptr<mp::QemuImgProcessSpec> make_capture_spec(const std::string& tag,
const std::filesystem::path& image_path)
{
return std::make_unique<mp::QemuImgProcessSpec>(
QStringList{"snapshot", "-c", tag, MP_PLATFORM.path_to_qstr(image_path)},
QStringList{"snapshot",
"-c",
QString::fromStdString(tag),
MP_PLATFORM.path_to_qstr(image_path)},
/* src_img = */ "",
image_path);
}

std::unique_ptr<mp::QemuImgProcessSpec> make_restore_spec(const QString& tag,
std::unique_ptr<mp::QemuImgProcessSpec> make_restore_spec(const std::string& tag,
const std::filesystem::path& image_path)
{
return std::make_unique<mp::QemuImgProcessSpec>(
QStringList{"snapshot", "-a", tag, MP_PLATFORM.path_to_qstr(image_path)},
QStringList{"snapshot",
"-a",
QString::fromStdString(tag),
MP_PLATFORM.path_to_qstr(image_path)},
/* src_img = */ "",
image_path);
}

std::unique_ptr<mp::QemuImgProcessSpec> make_delete_spec(const QString& tag,
std::unique_ptr<mp::QemuImgProcessSpec> make_delete_spec(const std::string& tag,
const std::filesystem::path& image_path)
{
return std::make_unique<mp::QemuImgProcessSpec>(
QStringList{"snapshot", "-d", tag, MP_PLATFORM.path_to_qstr(image_path)},
QStringList{"snapshot",
"-d",
QString::fromStdString(tag),
MP_PLATFORM.path_to_qstr(image_path)},
/* src_img = */ "",
image_path);
}
Expand All @@ -73,7 +82,7 @@ mp::QemuSnapshot::QemuSnapshot(const std::string& name,
{
}

mp::QemuSnapshot::QemuSnapshot(const QString& filename,
mp::QemuSnapshot::QemuSnapshot(const std::filesystem::path& filename,
QemuVirtualMachine& vm,
VirtualMachineDescription& desc)
: BaseSnapshot{filename, vm, desc}, desc{desc}, image_path{desc.image.image_path}
Expand Down
6 changes: 3 additions & 3 deletions src/platform/backends/qemu/qemu_snapshot.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@

#include <shared/base_snapshot.h>

#include <multipass/path.h>

namespace multipass
{
class QemuVirtualMachine;
Expand All @@ -38,7 +36,9 @@ class QemuSnapshot : public BaseSnapshot
const VMSpecs& specs,
QemuVirtualMachine& vm,
VirtualMachineDescription& desc);
QemuSnapshot(const QString& filename, QemuVirtualMachine& vm, VirtualMachineDescription& desc);
QemuSnapshot(const std::filesystem::path& filename,
QemuVirtualMachine& vm,
VirtualMachineDescription& desc);

protected:
void capture_impl() override;
Expand Down
2 changes: 1 addition & 1 deletion src/platform/backends/qemu/qemu_virtual_machine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,7 @@ bool multipass::QemuVirtualMachine::unplugged()
auto mp::QemuVirtualMachine::make_specific_snapshot(const QString& filename)
-> std::shared_ptr<Snapshot>
{
return std::make_shared<QemuSnapshot>(filename, *this, desc);
return std::make_shared<QemuSnapshot>(MP_PLATFORM.qstr_to_path(filename), *this, desc);
}

void mp::QemuVirtualMachine::fetch_ip(std::chrono::milliseconds timeout)
Expand Down
78 changes: 37 additions & 41 deletions src/platform/backends/shared/base_snapshot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,18 @@

#include <multipass/file_ops.h>
#include <multipass/json_utils.h>
#include <multipass/platform.h>
#include <multipass/virtual_machine_description.h>
#include <multipass/vm_mount.h>
#include <multipass/vm_specs.h>
#include <scope_guard.hpp>

#include <QString>

#include <QFile>
#include <QString>
#include <QTemporaryDir>

#include <fmt/format.h>

#include <stdexcept>

namespace mp = multipass;
Expand All @@ -37,41 +40,33 @@ namespace
{
constexpr auto snapshot_extension = "snapshot.json";
constexpr auto index_digits = 4; // these two go together
const auto snapshot_template =
QStringLiteral("@s%1"); /* avoid confusion with snapshot names by prepending a character
that can't be part of the name (users can call a snapshot
"s1", but they cannot call it "@s1") */

QString derive_index_string(int index)
{
return QString{"%1"}.arg(index, index_digits, 10, QLatin1Char('0'));
}
// Avoid confusion with snapshot names by prepending a character that can't be part of the name
// (users can call a snapshot "s1", but they cannot call it "@s1").
constexpr auto snapshot_template = "@s{}";

mp::SnapshotDescription read_snapshot_json(const QString& filename,
mp::SnapshotDescription read_snapshot_json(const std::filesystem::path& filename,
const mp::VirtualMachine& vm,
const mp::VirtualMachineDescription& vm_desc)
{
QFile file{filename};
if (!MP_FILEOPS.open(file, QIODevice::ReadOnly))
throw std::runtime_error{
fmt::format("Could not open snapshot file for for reading: {}", file.fileName())};

const auto& data = MP_FILEOPS.read_all(file);
if (data.isEmpty())
throw std::runtime_error{fmt::format("Empty snapshot JSON: {}", file.fileName())};

try
if (auto data = MP_FILEOPS.try_read_file(filename))
{
const auto json = boost::json::parse(std::string_view(data));
return value_to<mp::SnapshotDescription>(json.at("snapshot"),
mp::SnapshotContext{vm, vm_desc});
}
catch (const boost::system::system_error& e)
{
throw std::runtime_error{fmt::format("Could not parse snapshot JSON; error: {}; file: {}",
e.what(),
file.fileName())};
try
{
const auto json = boost::json::parse(*data);
return value_to<mp::SnapshotDescription>(json.at("snapshot"),
mp::SnapshotContext{vm, vm_desc});
}
catch (const boost::system::system_error& e)
{
throw std::runtime_error{
fmt::format("Could not parse snapshot JSON; error: {}; file: {}",
e.what(),
filename)};
}
}

throw std::runtime_error{
fmt::format("Could not open snapshot file for for reading: {}", filename)};
}

std::shared_ptr<mp::Snapshot> find_parent(const mp::SnapshotDescription& desc,
Expand All @@ -97,8 +92,8 @@ mp::BaseSnapshot::BaseSnapshot(SnapshotDescription desc,
bool captured)
: desc{std::move(desc)},
parent{std::move(parent)},
id{snapshot_template.arg(this->desc.index)},
storage_dir{vm.instance_directory()},
id{fmt::format(snapshot_template, this->desc.index)},
storage_dir{MP_PLATFORM.qstr_to_path(vm.instance_directory().path())},
captured{captured}
{
this->desc.parent_index = this->parent ? this->parent->get_index() : 0;
Expand All @@ -108,8 +103,8 @@ mp::BaseSnapshot::BaseSnapshot(SnapshotDescription desc,

mp::BaseSnapshot::BaseSnapshot(SnapshotDescription desc, VirtualMachine& vm, bool captured)
: desc{std::move(desc)},
id{snapshot_template.arg(desc.index)},
storage_dir{vm.instance_directory()},
id{fmt::format(snapshot_template, desc.index)},
storage_dir{MP_PLATFORM.qstr_to_path(vm.instance_directory().path())},
captured{captured}
{
parent = find_parent(this->desc, vm);
Expand Down Expand Up @@ -143,7 +138,7 @@ mp::BaseSnapshot::BaseSnapshot(const std::string& name,
{
}

mp::BaseSnapshot::BaseSnapshot(const QString& filename,
mp::BaseSnapshot::BaseSnapshot(const std::filesystem::path& filename,
VirtualMachine& vm,
const VirtualMachineDescription& desc)
: BaseSnapshot{read_snapshot_json(filename, vm, desc), vm, /*captured=*/true}
Expand All @@ -155,7 +150,7 @@ void mp::BaseSnapshot::persist() const
assert(captured && "precondition: only captured snapshots can be persisted");
const std::unique_lock lock{mutex};

auto snapshot_filepath = storage_dir.filePath(derive_snapshot_filename());
auto snapshot_filepath = storage_dir / derive_snapshot_filename();
boost::json::value json = {{"snapshot", boost::json::value_from(desc)}};
MP_FILEOPS.write_transactionally(snapshot_filepath, pretty_print(json));
}
Expand All @@ -167,8 +162,9 @@ auto mp::BaseSnapshot::erase_helper()
if (!tmp_dir->isValid())
throw std::runtime_error{"Could not create temporary directory"};

const auto snapshot_filename = derive_snapshot_filename();
auto snapshot_filepath = storage_dir.filePath(snapshot_filename);
const auto snapshot_filename = QString::fromStdString(derive_snapshot_filename());
QDir qstorage_dir = MP_PLATFORM.path_to_qstr(storage_dir);
auto snapshot_filepath = qstorage_dir.filePath(snapshot_filename);
auto deleting_filepath = tmp_dir->filePath(snapshot_filename);

QFile snapshot_file{snapshot_filepath};
Expand All @@ -195,7 +191,7 @@ void mp::BaseSnapshot::erase()
rollback_snapshot_file.dismiss();
}

QString mp::BaseSnapshot::derive_snapshot_filename() const
std::string mp::BaseSnapshot::derive_snapshot_filename() const
{
return QString{"%1.%2"}.arg(derive_index_string(desc.index), snapshot_extension);
return fmt::format("{0:0{1}}.{2}", desc.index, index_digits, snapshot_extension);
}
Loading
Loading