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] Fix QEMU backend refusing available snapshot names #3158

Merged
merged 3 commits into from
Jul 12, 2023

Conversation

ricab
Copy link
Collaborator

@ricab ricab commented Jul 11, 2023

Fix QEMU backend refusing available snapshot names that are also prefixes in existing snapshot names.

When looking for existing snapshot names, look for whitespace-terminated
names only, to avoid matching on prefixes.
Avoid intermediate strings when looking for existing snapshot names.
@ricab ricab requested a review from sharder996 July 11, 2023 18:38
Copy link
Contributor

@sharder996 sharder996 left a comment

Choose a reason for hiding this comment

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

LGTM!

@sharder996 sharder996 self-requested a review July 11, 2023 21:38
Copy link
Contributor

@sharder996 sharder996 left a comment

Choose a reason for hiding this comment

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

Actually, there seems to be a few tests that are failing now

[  FAILED  ] 3 tests, listed below:
[  FAILED  ] QemuBackend.verify_qemu_arguments_when_resuming_suspend_image
[  FAILED  ] QemuBackend.verify_qemu_arguments_when_resuming_suspend_image_uses_metadata
[  FAILED  ] QemuBackend.verify_qemu_arguments_from_metadata_are_used

@ricab
Copy link
Collaborator Author

ricab commented Jul 11, 2023

Yeah, I noticed that too. I will look into it tomorrow.

Fix failing tests by adapting to the QEMU backend now expecting the
snapshot tag column to end in whitespace.
@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Merging #3158 (f2b87f8) into snapshots (b7f4682) will decrease coverage by 0.20%.
The diff coverage is 43.32%.

@@              Coverage Diff              @@
##           snapshots    #3158      +/-   ##
=============================================
- Coverage      85.07%   84.88%   -0.20%     
=============================================
  Files            246      246              
  Lines          12978    13016      +38     
=============================================
+ Hits           11041    11048       +7     
- Misses          1937     1968      +31     
Impacted Files Coverage Δ
include/multipass/exceptions/snapshot_exceptions.h 0.00% <0.00%> (ø)
include/multipass/virtual_machine.h 100.00% <ø> (ø)
include/multipass/vm_mount.h 44.44% <0.00%> (-12.70%) ⬇️
src/client/cli/cmd/delete.h 100.00% <ø> (ø)
src/daemon/daemon.h 100.00% <ø> (ø)
src/daemon/daemon_rpc.cpp 87.87% <0.00%> (ø)
src/daemon/vm_specs.h 57.14% <0.00%> (-22.86%) ⬇️
...tform/backends/libvirt/libvirt_virtual_machine.cpp 62.88% <0.00%> (ø)
src/platform/backends/lxd/lxd_mount_handler.h 0.00% <ø> (ø)
src/platform/backends/lxd/lxd_virtual_machine.cpp 89.21% <0.00%> (ø)
... and 34 more

@ricab
Copy link
Collaborator Author

ricab commented Jul 12, 2023

It should be good now @sharder996.

Copy link
Contributor

@sharder996 sharder996 left a comment

Choose a reason for hiding this comment

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

Looks good now, thanks!

@sharder996 sharder996 merged commit e68d186 into snapshots Jul 12, 2023
10 of 13 checks passed
@bors bors bot deleted the fix-refused-snapshot-name branch July 12, 2023 16:26
ricab pushed a commit that referenced this pull request Aug 29, 2023
[snapshots] Fix QEMU backend refusing available snapshot names
ricab pushed a commit that referenced this pull request Oct 25, 2023
[snapshots] Fix QEMU backend refusing available snapshot names
ricab pushed a commit that referenced this pull request Oct 30, 2023
[snapshots] Fix QEMU backend refusing available snapshot names
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants