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.

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

[snapshots] resolve instance dir in vm #3130

Merged
merged 35 commits into from
Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
0da1f3d
[image-vault] drive by fix pass by value
sharder996 Jun 1, 2023
6cc1de0
[image-vault] drive by move functions in unnamed namespace together
sharder996 Jun 1, 2023
0180540
[vm] have vms be aware of where they are stored
sharder996 Jun 16, 2023
44746a7
[vm] reference internally stored instance directory location
sharder996 Jun 16, 2023
9fd4194
[vm factory] drive by fix add const modifier
sharder996 Jun 16, 2023
1f1b9e9
[vm factory] give responsibility of instances dir to vm factory
sharder996 Jun 16, 2023
fe9634f
[vm factory] provide external access to specific vm instance directory
sharder996 Jun 16, 2023
05f9156
[vm factory] populate vm instance directory on creation
sharder996 Jun 16, 2023
7d79236
[vm factory] give responsibility of removing instance dir on deletion…
sharder996 Jun 16, 2023
e68df45
[image vault] have image vault reference factory for instance directo…
sharder996 Jun 19, 2023
f167348
[tests] Adapt LXD mount tests to instance_dir
ricab Jun 27, 2023
a03e950
[image vault] rename parameter
sharder996 Aug 31, 2023
111f021
[vm factory] move common code to base class
sharder996 Aug 31, 2023
7552d76
[vm] remove extra constructor
sharder996 Aug 31, 2023
57868c2
[vm image vault] only create vm directory when required
sharder996 Aug 31, 2023
b15cc69
[vm] make instance_dir protected
sharder996 Aug 31, 2023
dec0b4d
[vm factory] use mp::Path for clarity on return parameter usage
sharder996 Aug 31, 2023
54faf5f
[vm factory] make constructor explicit
sharder996 Aug 31, 2023
30cbc96
[vm factory] use virtual{,-impl} architecture to remove duplicate fun…
sharder996 Sep 13, 2023
35c466c
[qemu] Fix circular dependency in initialization
ricab Sep 14, 2023
9ff29e0
[qemu] Remove unnecessary `explicit`
ricab Sep 14, 2023
cf88eef
[qemu] Make QemuPlatform::get_directory_name const
ricab Sep 14, 2023
84ac031
[tests] update tests to properly reference an instance's directory an…
sharder996 Sep 14, 2023
ff13fbc
[vm factory] make instance variable private
sharder996 Sep 14, 2023
04a4d7b
[lxd vm image vault] label unused parameter
sharder996 Sep 14, 2023
2020333
linting changes
sharder996 Sep 14, 2023
8e50791
[vms] Remove artificial constructors for tests
ricab Sep 18, 2023
3029016
[tests] Use a temporary dir in remaining stub VMs
ricab Sep 18, 2023
4788cf3
rename param to be more descriptive
sharder996 Sep 19, 2023
8a3f6a0
[lxd] edit log string to be more descriptive
sharder996 Sep 19, 2023
373a23d
remove unnecessary comment
sharder996 Sep 19, 2023
feb86ea
[vm factory] rename function
sharder996 Sep 19, 2023
33af0f6
[vm factory] extract instance directory derivation into util function
sharder996 Sep 20, 2023
400fbb3
[vm image vault] correct instance directory creation location
sharder996 Sep 20, 2023
463ef1c
[utils] avoid hardcoded directory separators
sharder996 Sep 20, 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
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,11 @@ mp::LibVirtVirtualMachineFactory::~LibVirtVirtualMachineFactory()
}
}

void mp::LibVirtVirtualMachineFactory::remove_resources_for(const std::string& name)
void mp::LibVirtVirtualMachineFactory::remove_resources_for_impl(const std::string& name)
{
auto connection = LibVirtVirtualMachine::open_libvirt_connection(libvirt_wrapper);

libvirt_wrapper->virDomainUndefine(libvirt_wrapper->virDomainLookupByName(connection.get(), name.c_str()));

BaseVirtualMachineFactory::remove_resources_for(name);
}

mp::VMImage mp::LibVirtVirtualMachineFactory::prepare_source_image(const VMImage& source_image)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ class LibVirtVirtualMachineFactory final : public BaseVirtualMachineFactory

VirtualMachine::UPtr create_virtual_machine(const VirtualMachineDescription& desc,
VMStatusMonitor& monitor) override;
void remove_resources_for(const std::string& name) override;
VMImage prepare_source_image(const VMImage& source_image) override;
void prepare_instance_image(const VMImage& instance_image, const VirtualMachineDescription& desc) override;
void hypervisor_health_check() override;
Expand All @@ -46,6 +45,9 @@ class LibVirtVirtualMachineFactory final : public BaseVirtualMachineFactory
// Making this public makes this modifiable which is necessary for testing
LibvirtWrapper::UPtr libvirt_wrapper;

protected:
void remove_resources_for_impl(const std::string& name) override;

private:
const Path data_dir;
std::string bridge_name;
Expand Down
3 changes: 1 addition & 2 deletions src/platform/backends/lxd/lxd_virtual_machine_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,9 @@ mp::VirtualMachine::UPtr mp::LXDVirtualMachineFactory::create_virtual_machine(co
MP_UTILS.make_dir(get_instance_directory_name(desc.vm_name)));
ricab marked this conversation as resolved.
Show resolved Hide resolved
}

void mp::LXDVirtualMachineFactory::remove_resources_for(const std::string& name)
void mp::LXDVirtualMachineFactory::remove_resources_for_impl(const std::string& name)
{
mpl::log(mpl::Level::trace, category, fmt::format("No resources to remove for \"{}\"", name));
ricab marked this conversation as resolved.
Show resolved Hide resolved
BaseVirtualMachineFactory::remove_resources_for(name);
}

auto mp::LXDVirtualMachineFactory::prepare_source_image(const VMImage& source_image) -> VMImage
Expand Down
4 changes: 3 additions & 1 deletion src/platform/backends/lxd/lxd_virtual_machine_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ class LXDVirtualMachineFactory : public BaseVirtualMachineFactory
void prepare_networking(std::vector<NetworkInterface>& extra_interfaces) override;
VirtualMachine::UPtr create_virtual_machine(const VirtualMachineDescription& desc,
VMStatusMonitor& monitor) override;
void remove_resources_for(const std::string& name) override;
VMImage prepare_source_image(const VMImage& source_image) override;
void prepare_instance_image(const VMImage& instance_image, const VirtualMachineDescription& desc) override;
void hypervisor_health_check() override;
Expand All @@ -53,6 +52,9 @@ class LXDVirtualMachineFactory : public BaseVirtualMachineFactory

std::vector<NetworkInterfaceInfo> networks() const override;

protected:
void remove_resources_for_impl(const std::string& name) override;

protected:
std::string create_bridge_with(const NetworkInterfaceInfo& interface) override;

Expand Down
3 changes: 1 addition & 2 deletions src/platform/backends/qemu/qemu_virtual_machine_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,9 @@ mp::VirtualMachine::UPtr mp::QemuVirtualMachineFactory::create_virtual_machine(c
MP_UTILS.make_dir(get_instance_directory_name(desc.vm_name)));
ricab marked this conversation as resolved.
Show resolved Hide resolved
}

void mp::QemuVirtualMachineFactory::remove_resources_for(const std::string& name)
void mp::QemuVirtualMachineFactory::remove_resources_for_impl(const std::string& name)
{
qemu_platform->remove_resources_for(name);
BaseVirtualMachineFactory::remove_resources_for(name);
}

mp::VMImage mp::QemuVirtualMachineFactory::prepare_source_image(const mp::VMImage& source_image)
Expand Down
4 changes: 3 additions & 1 deletion src/platform/backends/qemu/qemu_virtual_machine_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,16 @@ class QemuVirtualMachineFactory final : public BaseVirtualMachineFactory

VirtualMachine::UPtr create_virtual_machine(const VirtualMachineDescription& desc,
VMStatusMonitor& monitor) override;
void remove_resources_for(const std::string& name) override;
VMImage prepare_source_image(const VMImage& source_image) override;
void prepare_instance_image(const VMImage& instance_image, const VirtualMachineDescription& desc) override;
void hypervisor_health_check() override;
QString get_backend_version_string() const override;
QString get_backend_directory_name() const override;
std::vector<NetworkInterfaceInfo> networks() const override;

protected:
void remove_resources_for_impl(const std::string& name) override;

private:
explicit QemuVirtualMachineFactory(QemuPlatform::UPtr qemu_platform, const Path& data_dir);

Expand Down
6 changes: 0 additions & 6 deletions src/platform/backends/shared/base_virtual_machine_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,6 @@ auto find_bridge_with(const NetworkContainer& networks, const std::string& membe

mp::BaseVirtualMachineFactory::BaseVirtualMachineFactory(const Path& instances_dir) : instances_dir{instances_dir} {};

void mp::BaseVirtualMachineFactory::remove_resources_for(const std::string& name)
{
QDir instance_dir{get_instance_directory_name(name)};
instance_dir.removeRecursively();
}

void mp::BaseVirtualMachineFactory::configure(VirtualMachineDescription& vm_desc)
{
auto instance_dir{mpu::base_dir(vm_desc.image.image_path)};
Expand Down
11 changes: 10 additions & 1 deletion src/platform/backends/shared/base_virtual_machine_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class BaseVirtualMachineFactory : public VirtualMachineFactory
BaseVirtualMachineFactory() = default;
ricab marked this conversation as resolved.
Show resolved Hide resolved
explicit BaseVirtualMachineFactory(const Path& instances_dir);

void remove_resources_for(const std::string& name) override;
void remove_resources_for(const std::string& name) final;

FetchType fetch_type() override
{
Expand Down Expand Up @@ -86,8 +86,17 @@ class BaseVirtualMachineFactory : public VirtualMachineFactory
virtual void prepare_interface(NetworkInterface& net, std::vector<NetworkInterfaceInfo>& host_nets,
const std::string& bridge_type);

virtual void remove_resources_for_impl(const std::string& name) = 0;
ricab marked this conversation as resolved.
Show resolved Hide resolved

Path instances_dir;
ricab marked this conversation as resolved.
Show resolved Hide resolved
};
} // namespace multipass

inline void multipass::BaseVirtualMachineFactory::remove_resources_for(const std::string& name)
{
remove_resources_for_impl(name);
QDir instance_dir{get_instance_directory_name(name)};
instance_dir.removeRecursively();
}

#endif // MULTIPASS_BASE_VIRTUAL_MACHINE_FACTORY_H
2 changes: 1 addition & 1 deletion tests/stub_virtual_machine_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ struct StubVirtualMachineFactory : public multipass::BaseVirtualMachineFactory
return std::make_unique<StubVirtualMachine>();
}

void remove_resources_for(const std::string& name) override
void remove_resources_for_impl(const std::string& name) override
{
}

Expand Down
2 changes: 1 addition & 1 deletion tests/test_base_virtual_machine_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ struct MockBaseFactory : mp::BaseVirtualMachineFactory
{
MOCK_METHOD(mp::VirtualMachine::UPtr, create_virtual_machine,
(const mp::VirtualMachineDescription&, mp::VMStatusMonitor&), (override));
MOCK_METHOD(void, remove_resources_for, (const std::string&), (override));
MOCK_METHOD(mp::VMImage, prepare_source_image, (const mp::VMImage&), (override));
MOCK_METHOD(void, prepare_instance_image, (const mp::VMImage&, const mp::VirtualMachineDescription&), (override));
MOCK_METHOD(void, hypervisor_health_check, (), (override));
Expand All @@ -51,6 +50,7 @@ struct MockBaseFactory : mp::BaseVirtualMachineFactory
(mp::NetworkInterface & net, std::vector<mp::NetworkInterfaceInfo>& host_nets,
const std::string& bridge_type),
(override));
MOCK_METHOD(void, remove_resources_for_impl, (const std::string&), (override));

std::string base_create_bridge_with(const mp::NetworkInterfaceInfo& interface)
{
Expand Down