Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[snapshots] record snapshot creation time #3125

Merged
merged 7 commits into from
Jun 28, 2023
Merged

[snapshots] record snapshot creation time #3125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c0be508
[snapshot] add creation timestamp field to snapshot metadata
sharder996 Jun 13, 2023
3eef414
[daemon] populate info reply with snapshot creation time
sharder996 Jun 13, 2023
661ac5c
[snapshot] remove unneeded setter and set member variable to const
sharder996 Jun 23, 2023
a46ae74
[snapshot] move creation timestamp to base class
sharder996 Jun 23, 2023
987b7d1
[review] formatting changes
sharder996 Jun 23, 2023
b495ba3
[snapshot] move const members together
sharder996 Jun 28, 2023
976eb07
[snapshot] include ms in recorded snapshot time
sharder996 Jun 28, 2023
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
2 changes: 2 additions & 0 deletions include/multipass/snapshot.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <unordered_map>

class QJsonObject;
class QDateTime;

namespace multipass
{
Expand All @@ -40,6 +41,7 @@ class Snapshot : private DisabledCopyMove
virtual std::string get_name() const = 0;
virtual std::string get_comment() const = 0;
virtual std::string get_parent_name() const = 0;
virtual QDateTime get_creation_timestamp() const = 0;
ricab marked this conversation as resolved.
Show resolved Hide resolved
virtual std::shared_ptr<const Snapshot> get_parent() const = 0;
virtual std::shared_ptr<Snapshot> get_parent() = 0;

Expand Down
5 changes: 4 additions & 1 deletion src/daemon/daemon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1761,7 +1761,10 @@
fundamentals->set_snapshot_name(snapshot->get_name());
fundamentals->set_parent(snapshot->get_parent_name());
fundamentals->set_comment(snapshot->get_comment());
// TODO@snapshots populate snapshot creation time once available

auto timestamp = fundamentals->mutable_creation_timestamp();
timestamp->set_seconds(snapshot->get_creation_timestamp().toSecsSinceEpoch());
timestamp->set_nanos(snapshot->get_creation_timestamp().time().msec() * 1'000'000);

Check warning on line 1767 in src/daemon/daemon.cpp

View check run for this annotation

Codecov / codecov/patch

src/daemon/daemon.cpp#L1765-L1767 

Added lines #L1765 - L1767 were not covered by tests
};

if (const auto& it = instance_snapshots_map.find(name);
Expand Down
23 changes: 18 additions & 5 deletions src/platform/backends/shared/base_snapshot.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,14 @@
}
} // namespace

mp::BaseSnapshot::BaseSnapshot(const std::string& name, const std::string& comment, int num_cores, MemorySize mem_size,
mp::BaseSnapshot::BaseSnapshot(const std::string& name, const std::string& comment, // NOLINT(modernize-pass-by-value)

Check warning on line 81 in src/platform/backends/shared/base_snapshot.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/shared/base_snapshot.cpp#L81 

Added line #L81 was not covered by tests
const QDateTime& creation_timestamp, int num_cores, MemorySize mem_size,
MemorySize disk_space, VirtualMachine::State state,
std::unordered_map<std::string, VMMount> mounts, QJsonObject metadata,
std::shared_ptr<Snapshot> parent)
: name{name},
comment{comment},
creation_timestamp{creation_timestamp},

Check warning on line 88 in src/platform/backends/shared/base_snapshot.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/shared/base_snapshot.cpp#L88 

Added line #L88 was not covered by tests
num_cores{num_cores},
mem_size{mem_size},
disk_space{disk_space},
Expand All @@ -104,8 +106,16 @@

mp::BaseSnapshot::BaseSnapshot(const std::string& name, const std::string& comment, const VMSpecs& specs,
std::shared_ptr<Snapshot> parent)
: BaseSnapshot{name, comment, specs.num_cores, specs.mem_size, specs.disk_space,
specs.state, specs.mounts, specs.metadata, std::move(parent)}
: BaseSnapshot{name,
comment,
QDateTime::currentDateTimeUtc(),
specs.num_cores,

Check warning on line 112 in src/platform/backends/shared/base_snapshot.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/shared/base_snapshot.cpp#L111-L112 

Added lines #L111 - L112 were not covered by tests
specs.mem_size,
specs.disk_space,
specs.state,
specs.mounts,
specs.metadata,
std::move(parent)}

Check warning on line 118 in src/platform/backends/shared/base_snapshot.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/shared/base_snapshot.cpp#L115-L118 

Added lines #L115 - L118 were not covered by tests
{
}

Expand All @@ -115,8 +125,10 @@
}

mp::BaseSnapshot::BaseSnapshot(InnerJsonTag, const QJsonObject& json, VirtualMachine& vm)
: BaseSnapshot{json["name"].toString().toStdString(), // name
json["comment"].toString().toStdString(), // comment
: BaseSnapshot{json["name"].toString().toStdString(), // name
json["comment"].toString().toStdString(), // comment
QDateTime::fromString(json["creation_timestamp"].toString(),

Check warning on line 130 in src/platform/backends/shared/base_snapshot.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/shared/base_snapshot.cpp#L128-L130 

Added lines #L128 - L130 were not covered by tests
Qt::ISODateWithMs), // creation_timestamp
json["num_cores"].toInt(), // num_cores
MemorySize{json["mem_size"].toString().toStdString()}, // mem_size
MemorySize{json["disk_space"].toString().toStdString()}, // disk_space
Expand All @@ -134,6 +146,7 @@

snapshot.insert("name", QString::fromStdString(name));
snapshot.insert("comment", QString::fromStdString(comment));
snapshot.insert("creation_timestamp", creation_timestamp.toString(Qt::ISODateWithMs));

Check warning on line 149 in src/platform/backends/shared/base_snapshot.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/shared/base_snapshot.cpp#L149 

Added line #L149 was not covered by tests
snapshot.insert("num_cores", num_cores);
snapshot.insert("mem_size", QString::number(mem_size.in_bytes()));
snapshot.insert("disk_space", QString::number(disk_space.in_bytes()));
Expand Down
14 changes: 11 additions & 3 deletions src/platform/backends/shared/base_snapshot.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@

std::string get_name() const override;
std::string get_comment() const override;
QDateTime get_creation_timestamp() const override;
ricab marked this conversation as resolved.
Show resolved Hide resolved
std::string get_parent_name() const override;
std::shared_ptr<const Snapshot> get_parent() const override;
std::shared_ptr<Snapshot> get_parent() override;
Expand Down Expand Up @@ -73,15 +74,17 @@
{
};
BaseSnapshot(InnerJsonTag, const QJsonObject& json, VirtualMachine& vm);
BaseSnapshot(const std::string& name, const std::string& comment, int num_cores, MemorySize mem_size,
MemorySize disk_space, VirtualMachine::State state, std::unordered_map<std::string, VMMount> mounts,
QJsonObject metadata, std::shared_ptr<Snapshot> parent);
BaseSnapshot(const std::string& name, const std::string& get_comment, const QDateTime& creation_timestamp,
int num_cores, MemorySize mem_size, MemorySize disk_space, VirtualMachine::State state,
std::unordered_map<std::string, VMMount> mounts, QJsonObject metadata,
std::shared_ptr<Snapshot> parent);

private:
std::string name;
std::string comment;

// This class is non-copyable and having these const simplifies thread safety
const QDateTime creation_timestamp; // NOLINT(cppcoreguidelines-avoid-const-or-ref-data-members)
const int num_cores; // NOLINT(cppcoreguidelines-avoid-const-or-ref-data-members)
const MemorySize mem_size; // NOLINT(cppcoreguidelines-avoid-const-or-ref-data-members)
const MemorySize disk_space; // NOLINT(cppcoreguidelines-avoid-const-or-ref-data-members)
Expand All @@ -107,6 +110,11 @@
return comment;
}

inline QDateTime multipass::BaseSnapshot::get_creation_timestamp() const

Check warning on line 113 in src/platform/backends/shared/base_snapshot.h

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/shared/base_snapshot.h#L113 

Added line #L113 was not covered by tests
{
return creation_timestamp;

Check warning on line 115 in src/platform/backends/shared/base_snapshot.h

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/shared/base_snapshot.h#L115 

Added line #L115 was not covered by tests
}

inline std::string multipass::BaseSnapshot::get_parent_name() const
{
std::unique_lock lock{mutex};
Expand Down
5 changes: 5 additions & 0 deletions tests/stub_snapshot.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ struct StubSnapshot : public Snapshot
return {};
}

QDateTime get_creation_timestamp() const noexcept override
{
return QDateTime{};
}

std::string get_parent_name() const override
{
return {};
Expand Down