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
Multipass clone #3264
Draft
georgeliao
wants to merge
110
commits into
main
Choose a base branch
from
multipass_clone
base: main
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Multipass clone #3264
+1,113
−31
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
georgeliao
force-pushed
the
multipass_clone
branch
from
November 2, 2023 08:41
d8739fd
to
c9f71b0
Compare
georgeliao
force-pushed
the
multipass_clone
branch
from
November 16, 2023 11:46
0e5a833
to
5e2a139
Compare
georgeliao
force-pushed
the
multipass_clone
branch
from
December 11, 2023 14:23
77e02fb
to
69d81b9
Compare
georgeliao
force-pushed
the
multipass_clone
branch
2 times, most recently
from
January 3, 2024 10:04
5b2c7f7
to
2bb5b3a
Compare
georgeliao
force-pushed
the
multipass_clone
branch
from
January 12, 2024 14:20
eddafcc
to
fb663a5
Compare
georgeliao
force-pushed
the
multipass_clone
branch
3 times, most recently
from
February 6, 2024 15:48
794467a
to
dac94dd
Compare
georgeliao
force-pushed
the
multipass_clone
branch
6 times, most recently
from
February 20, 2024 11:46
563f9f3
to
f448929
Compare
georgeliao
force-pushed
the
multipass_clone
branch
5 times, most recently
from
March 5, 2024 12:06
c28404e
to
a662819
Compare
georgeliao
force-pushed
the
multipass_clone
branch
from
March 7, 2024 20:51
f96a5cf
to
a662819
Compare
georgeliao
force-pushed
the
multipass_clone
branch
3 times, most recently
from
April 15, 2024 14:21
6b0b4c7
to
963f599
Compare
georgeliao
force-pushed
the
multipass_clone
branch
3 times, most recently
from
May 1, 2024 09:47
560583e
to
8c70116
Compare
georgeliao
force-pushed
the
multipass_clone
branch
from
May 8, 2024 10:02
92819f9
to
f328b81
Compare
…from QJsonObject to QJsonValue for consistency.
…re branch of cmd dispatch.
…rce and destination instance folders.
georgeliao
force-pushed
the
multipass_clone
branch
from
May 24, 2024 14:22
0cacea2
to
b059d3c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please read the spec first to get a sense of the context.
The code can be divided into several modules:
daemon::clone
method indaemon.cpp
,2.1. generate destination name, this step processes the name options of the input, use the given name or generate a new one if no name is provided. This step also needs the name list of instances to check if the name exist, it also requires the source spec
VMSpecs::clone_count
field to generate a new name.2.2. Once the new destination name is produced, we can clone the vmspec object, this step requires copying the vmspec instance and updating the
VMSpecs::default_mac_address
andVMSpecs::extra_interfaces
fields. Ideally, this method can be a data member method (calledclone
maybe) inVMSpecs
class. However, given the fact that the dependency code (generate_unused_mac_address
method andallocated_mac_addrs
data) still resides indaemon.cpp
, soclone_spec
is a local lambda method indaemon.cpp
for now.2.3. clone image record through the
VMImageVault::clone
method.2.4. calls
create_vm_and_clone_instance_dir_data
fromVirtualMachineFactory
.load_snapshots_and_update_unique_identifiers
is added tobase_snap_shot.cpp
, it loads the snapshot files and updates the cloud_init_instance_id item, and file path, default mac address, and extra interface mac address of the metadata in the case of qemu.qemu_virtual_machine_factory::create_vm_and_clone_instance_dir_data
, it copies the whole instance folder and updates the unique identifiers incloud-init-config.iso
file. Besides that, it creates the newVirtualMachine
instance and also loads and modifies snapshot files.multipass clone
command line.Functional testing should cover at least the following scenarios:
Small note:
The unit tests are not finished yet considering that they might change if the code structure is required to change according to the review.
Further thoughts on the code architecture (open for discussion):
VMSpecs::clone_count
to count cloned instances and theBaseVirtualMachine::snapshot_count
to count snapshots are cumbersome. We need a variable and the file or json item to track the value. If we change the name generation scheme to seeking the next available integer.multipassd-vm-instances.json
file themetadata::arguments
contains the image and clou-init file absolute path, themultipassd-instance-image-records.json
file has the path item points to the image file path. These paths in a way added more unnecessary instance name occurrences (the instance folder name in the path) which need to be updated during the clone or renaming process. Maybe a better disk data layout and C++ code structure can truly remove this. For example, if we split these json files into one item per file and move that into the instance directory, and meanwhile we can moveVMSpecs
intoVirtualMachine
class. With this new data layout and code structure, theVMSpecs
can access the instance directory and further access the multipassd-vm-instances.json file (with his own item), besides that, the data sync between theVMSpecs
andVirtualMachine
class becomes much easier. On top of these, there are more benefits like a rollback mechanism and thread-safety can be much easier.Mac_address_generator
class that can be accessed from everywhere and refactor the corresponding code. If this can be done, then theclone_spec
does not have to be a local lambda indaemon.cpp
.