LVM performance, supported services listing, misc fixes and tests #238
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
Make PyCharm understand what mixin those objects are for.
Remove unused imports and unused variables, add some more docstrings.
Network tests create own temporary netvm. Make it disconnected from the real netvm, to not interefere in tests.
Searching based on class is used in many tests, searching by class, not only by name in wait_for_window will allow to reduce code duplication. While at it, improve it additionally: - avoid active waiting for window and use `xdotool search --sync` instead - return found window id - add wait_for_window_coro() for use where coroutine is needed - when waiting for window to disappear, check window id once and wait for that particular window to disappear (avoid xdotool race conditions on window enumeration) Besides reducing code duplication, this also move various xdotool imperfections handling into one place.
Replace manual `xdotool search calls` with wait_for_window(), where compatible.
Make sure events are sent to specific window found with xdotool search, not the one having the focus. In case of Whonix, it can be first connection wizard or whonixcheck report.
1GB image easily exceed available space on openQA instances. Use 100MB instead.
R3 format had limitation of ~40 rules per VM. Do not generate compat rules (possibly hitting that limitation) if new format, free of that limitation is supported. Fixes QubesOS/qubes-issues#1570 Fixes QubesOS/qubes-issues#4228
It can be leftover from previous failed attempt. Don't crash on it, and replace it instead. QubesOS/qubes-issues#3438
Make it clear that volume creation fails because it needs to be in the same pool as its parent. This message is shown in context of `qvm-create -p root=MyPool` for example and the previous message didn't make sense at all. Fixes QubesOS/qubes-issues#3438
Codecov Report
@@ Coverage Diff @@
## master #238 +/- ##
==========================================
+ Coverage 58.09% 58.37% +0.28%
==========================================
Files 57 57
Lines 9275 9379 +104
==========================================
+ Hits 5388 5475 +87
- Misses 3887 3904 +17
Continue to review full report at Codecov.
|
marmarek
force-pushed
the
devel-20181015
branch
2 times, most recently
from
389e71f to
e4f26b7
Compare
Support 'supported-service.*' features requests coming from VMs. Set such features directly (allow only value '1') and remove any not reported in given call. This way uninstalling package providing given service will automatically remove related 'supported-service...' feature. Fixes QubesOS/qubes-issues#4402
On some storage pools this operation can also be time consuming - for example require creating temporary volume, and volume.create() already can be a coroutine. This is also requirement for making common code used by start()/create() etc be a coroutine, otherwise neither of them can be and will block other operations. Related to QubesOS/qubes-issues#4283
Both vm.create_on_disk() and vm.start() are coroutines. Tests in this class didn't run them, so basically didn't test anything. Wrap couroutine calls with self.loop.run_until_complete(). Additionally, don't fail if LVM pool is named differently. In that case, the test is rather sily, as it probably use the same pool for source and destination (operation already tested elsewhere). But it isn't a reason for failing the test.
LVM operations can take significant amount of time. This is especially visible when stopping a VM (`vm.storage.stop()`) - in that time the whole qubesd freeze for about 2 seconds. Fix this by making all the ThinVolume methods a coroutines (where supported). Each public coroutine is also wrapped with locking on volume._lock to avoid concurrency-related problems. This all also require changing internal helper functions to coroutines. There are two functions that still needs to be called from non-coroutine call sites: - init_cache/reset_cache (initial cache fill, ThinPool.setup()) - qubes_lvm (ThinVolume.export() So, those two functions need to live in two variants. Extract its common code to separate functions to reduce code duplications. Fixes QubesOS/qubes-issues#4283
marmarek
force-pushed
the
devel-20181015
branch
3 times, most recently
from
c37f144 to
23f46e9
Compare
vm.shutdown(wait=True) waited indefinitely for the shutdown, which makes useless without some boilerplate handling the timeout. Since the timeout may depend on the operating system inside, add a per-VM property for it, with value inheritance from template and then from global default_shutdown_timeout property. When timeout is reached, the method raises exception - whether to kill it or not is left to the caller. Fixes QubesOS/qubes-issues#1696
First unregister the domain from collection, and only then call remove_from_disk(). Removing it from collection prevent further calls being made to it. Or if anything else keep a reference to it (for example as a netvm), then abort the operation. Additionally this makes it unnecessary to take startup lock when cleaning it up in tests.
Cleaning up after domain shutdown (domain-stopped and domain-shutdown
events) relies on libvirt events which may be unreliable in some cases
(events may be processed with some delay, of if libvirt was restarted in
the meantime, may not happen at all). So, instead of ensuring only
proper ordering between shutdown cleanup and next startup, also trigger
the cleanup when we know for sure domain isn't running:
- at vm.kill() - after libvirt confirms domain was destroyed
- at vm.shutdown(wait=True) - after successful shutdown
- at vm.remove_from_disk() - after ensuring it isn't running but just
before actually removing it
This fixes various race conditions:
- qvm-kill && qvm-remove: remove could happen before shutdown cleanup
was done and storage driver would be confused about that
- qvm-shutdown --wait && qvm-clone: clone could happen before new content was
commited to the original volume, making the copy of previous VM state
(and probably more)
Previously it wasn't such a big issue on default configuration, because
LVM driver was fully synchronous, effectively blocking the whole qubesd
for the time the cleanup happened.
To avoid code duplication, factor out _ensure_shutdown_handled function
calling actual cleanup (and possibly canceling one called with libvirt
event). Note that now, "Duplicated stopped event from libvirt received!"
warning may happen in normal circumstances, not only because of some
bug.
It is very important that post-shutdown cleanup happen when domain is
not running. To ensure that, take startup_lock and under it 1) ensure
its halted and only then 2) execute the cleanup. This isn't necessary
when removing it from disk, because its already removed from the
collection at that time, which also avoids other calls to it (see also
"vm/dispvm: fix DispVM cleanup" commit).
Actually, taking the startup_lock in remove_from_disk function would
cause a deadlock in DispVM auto cleanup code:
- vm.kill (or other trigger for the cleanup)
- vm.startup_lock acquire <====
- vm._ensure_shutdown_handled
- domain-shutdown event
- vm._auto_cleanup (in DispVM class)
- vm.remove_from_disk
- cannot take vm.startup_lock again
marmarek
force-pushed
the
devel-20181015
branch
from
23f46e9 to
342d651
Compare
Cleanup VMs in template reverse topological order, not network one. Network can be set to None to break dependency, but template can't. For netvm to be changed, kill VMs first (kill doesn't check network dependency), so netvm change will not trigger side effects (runtime change, which could fail). This fixes cleanup for tests creating custom templates - previously order was undefined and if template was tried removed before its child VMs, it fails. All the relevant files were removed later anyway, but it lead to python objects leaks.
socat have only one variant, so one command line syntax to handle. It's also installed by default in Qubes VMs.
Yet another place wheren object references are leaked.
If domain is set to autostart, qubes-vm@ systemd service is used to start it at boot. Cleanup the service when domain is removed, and similarly enable the service when domain is created and already have autostart=True. Fixes QubesOS/qubes-issues#4014
Use something included in coreutils installed everywhere.
It's weird to set it for Windows, but not Linux.
Same as other vm-related errors. This helps QubesTestCase.cleanup_traceback() cleanup VM reference.
QubesOS/qubes-issues#3665 affects also tests...
Clean VM shutdown may timeout if its initiated before full startup, so make sure the full startup is completed first.
Nose loader do not provide loader.loadTestsFromTestCase(), use loader.loadTestsFromNames() instead.
First boot of whonix-ws based VM take extended period of time, because a lot of files needs to be copied to private volume. This takes even more time, when verbose logging through console is enabled. Extend the timeout for that.
marmarek
force-pushed
the
devel-20181015
branch
from
342d651 to
84c321b
Compare
…hown Try to collect more details about why the test failed. This will help only if qvm-open-in-dvm exist early. On the other hand, if it hang, or remote side fails to find the right editor (which results in GUI error message), this change will not provide any more details.
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.
Major changes:
during domain start/stop
Besides this, few minor fixes and a lot of improvements for system tests.
Fixes QubesOS/qubes-issues#4014
Fixes QubesOS/qubes-issues#1696
Fixes QubesOS/qubes-issues#4283
Fixes QubesOS/qubes-issues#2829
Fixes QubesOS/qubes-issues#4402
Fixes QubesOS/qubes-issues#3438
Fixes QubesOS/qubes-issues#1570
Fixes QubesOS/qubes-issues#4228