search

avocado-framework / avocado-vt

Avocado VT Plugin
https://avocado-vt.readthedocs.org/
Other
84 stars 243 forks source link

Is resetting the VM after taking a snapshot needed? #2660

Open samiraguiar opened 4 years ago

samiraguiar commented 4 years ago

In qemu_vm.py, after creating a snapshot in savevm() a "system_reset" command is sent to the VM monitor. I checked the commit that introduced this change but couldn't understand why.

I think this method was based on save_to_file(), which also sends a "system_reset" command. The message of the commit that refactored it says:

I chose to add a "system_reset" directly following the save operation because I cannot envision any situation where intentionally mismatching state with the save-file would be useful.

There's value in preventing testing accidents (saving rebuild time of guest-images or trying to detect corruption). Doing it this way also guarantees the post-save state will be "paused" which can be verified. Undefined state is much harder to verify.

"intentionally mismatching state with the save-file" seems to be what I need: I wanted to take a snapshot but keep the VM running for further operations, but it gets reset killing all the running applications. Granted, I can immediately call loadvm() right after savevm() (which is what the floppy test does), but maybe this constraint could be relaxed making the reset opt-out.

I also don't understand what is meant by "doing it this way also guarantees the post-save state will be 'paused'". Are there cases where the VM is restarted after the migration command is issued which are worked around by a system reset? If so, is this also applicable to the savevm() method? I was thinking that in case resetting avoids corruption, we could optionally load the saved state from savevm() itself to reduce side effects.

pevogam commented 3 years ago

@ypu, @lmr, @cevich Do you mind shedding some light on this source of confusion?

cevich commented 3 years ago

Oof...I remember last week somewhat clearly, last month less so, and years ago is really fuzzy :grin:

It's entirely possible those statements/assertions in question are due to old autotest and/or libvirt behaviors which are no-longer applicable. IIRC (very fuzzy), the save/load testing was very much hard-coupled to the migration testing. I would suggest simply going back to the libvirt docs, and confirming there are no statements to the effect of: "The VM state after save or migrate is 'undefined'". Assuming not, then making the reset optional (or removing it) from the harness is probably worth trying.