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

api/admin: fix backup exclude #202

Merged
merged 1 commit into from Mar 20, 2018
Merged

Conversation

rustybird
Copy link
Contributor

Bugfix on 59abdeb

@codecov
Copy link

codecov bot commented Mar 13, 2018

Codecov Report

Merging #202 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #202      +/-   ##
==========================================
- Coverage   55.53%   55.51%   -0.03%     
==========================================
  Files          56       56              
  Lines        9035     9035              
==========================================
- Hits         5018     5016       -2     
- Misses       4017     4019       +2
Flag Coverage Δ
#unittests 55.51% <100%> (-0.03%) ⬇️
Impacted Files Coverage Δ
qubes/api/admin.py 92.12% <100%> (ø) ⬆️
qubes/backup.py 40.04% <0%> (-0.24%) ⬇️
qubes/app.py 74.32% <0%> (-0.16%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 93bccf5...a0c5014. Read the comment docs.

# handle exclude
vms_to_backup.difference_update(vm for vm in self.app.domains
if any(qubes.utils.match_vm_name_with_special(vm, name)
for name in exclude_vms))
Copy link
Member

Choose a reason for hiding this comment

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

This is wrong, because exclude_vms argument is expected to be a list of VM names, but exclude in backup profile may also contain special keywords like @tag:something. Probably the easiest thing to do is to construct exclude list based on all VMs here (like it is here right now), but save it into exclude_vms instead of applying directly to vms_to_backup.

Copy link
Contributor Author

@rustybird rustybird Mar 20, 2018

Choose a reason for hiding this comment

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

Sorry, fixed!

test_621_qdb_vm_with_network is racy BTW.

@rustybird rustybird force-pushed the backup-exclude branch 2 times, most recently from 8fa784e to 45d1567 Compare March 20, 2018 02:26
@marmarek marmarek merged commit a0c5014 into QubesOS:master Mar 20, 2018
@rustybird rustybird deleted the backup-exclude branch September 12, 2018 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants