It has been broken for some time. People have blogged and posted about it, with these types of comments:
- 'It's broken!'
- 'I don't understand what it does.'
- 'It works like 'so', which is confusing, but that is just the way it is.'
- 'It works like 'so' and it is working correctly' (I haven't seen any of these, but people must believe this because the problem has yet to be fixed!)
1) To see the problem, perform the following steps:
- Create a new snapshot named: '1 January'
- Boot the VM and create a file called 'Work.txt', with this contents:
January
February
March
(This file represents changes done over 3 months since the 1st January snapshot.)
- Shutdown the VM. Suppose on 1 April the user wants to test an experimental change.
- Create a new snapshot named: 'Before 1 April Experiment', intended to protect the 'Work' while the experiment is done.
- Boot the VM and edit file 'Work.txt', adding 'Experimental change'.
- Shutdown the VM.
Suppose the user decides that the experiment was not wanted, and they want to undo the change.
They want to Revert the current state to what it was when the 'Before 1 April Experiment' snapshot was taken, and then delete that snapshot.
The VirtualBox UI provides Snapshot management buttons that can help the user unwind the change.
One such button is 'Discard Current Snapshot and State'.
Although the label on this button is ambigious, it seems that it will do what they want.
Hovering the mouse over this button displays the following information in the VirtualBox status bar:
'Discard the current snapshot and revert the machine to the state it had before the snapshot was taken'
Assuming the the 'current snapshot' is the most recent 1st April snapshot, then reverting the machine to the state it had before the 'Before 1 April Experiment' snapshot was taken is exactly what is required.
Just before the 1st April snapshot was taken, file 'Work.txt' had January/February/March in it, representing 3 months of work.
- Click the 'Discard Current Snapshot and State' button.
- Boot the VM and examine the contents of file 'Work.txt'. File WORK.TXT is not present!
The 'Discard Current Snapshot and State' button has not only reverted changes since the 1st April snapshot, and removed the 1st April snapshot, but it has ALSO reverted all changes since the 1st January snapshot, and erased 3 months of work!
This is a severe bug.
If this is not a bug (it is) then this is a severe design error.
(To people that might say: 'this is working as designed', I say: Rubbish! A feature that reverts back TWO snapshots is just silly and mostly useless. Why not go back 3 or more? Where is the useful feature that just goes back 1 snapshot and deletes that snapshot?)
NOTE: The 'Discard Current Snapshot and State' button acts differently depending upon if there are is ONLY ONE snapshot when it is run, or of there are 2 or more snapshots when it is run.
If the above steps are repeated, but excluding the creation of the '1 January' snapshot (start from a proper root hard disk) and ensure that the ONLY snapshot present is the 1st April one, then 'Discard Current Snapshot and State' performs as expected according to the status bar information text.
2) This different behaviour can be seen in the source code (see below).
'Revert to Current Snapshot' and 'Discard Current Snapshot and State' share some source code. The (pseudo) code looks like so:
Code: Select all
// http://www.virtualbox.org/browser/trunk/src/VBox/Main/MachineImpl.cpp?rev=13762#L8646
DiscardCurrentState() { // Implements 'Revert to Current Snapshot'
...
DiscardCurrentStateHandler(false); // discardCurSnapshot=false
...
}
// http://www.virtualbox.org/browser/trunk/src/VBox/Main/MachineImpl.cpp?rev=13762#L8710
DiscardCurrentSnapshotAndState() { // Implements 'Discard Current Snapshot and State'
...
DiscardCurrentStateHandler(true); // discardCurSnapshot=true
...
}
// http://www.virtualbox.org/browser/trunk/src/VBox/Main/MachineImpl.cpp?rev=13762#L9902
DiscardCurrentStateHandler(bool discardCurrentSnapshot) {
...
bool isLastSnapshot = CurrentSnapshot->parent().isNull(); // Is there ONE ONLY 'last' snapshot?
if (discardCurrentSnapshot && !isLastSnapshot) {
DiscardSnapshotHandler(CurrentSnapshot);
}
RevertToCurrentSnapShot(); // inline code to do so, not a function call...
if (discardCurrentSnapshot && isLastSnapshot) {
DiscardSnapshotHandler(CurrentSnapshot);
}
...
}- 'Revert to Current Snapshot' calls DiscardCurrentStateHandler(false), so that all that happens is:
RevertToCurrentSnapShot();
- 'Discard Current Snapshot and State', when the current snapshot has NO Parent, in other words there is ONLY one snapshot, does this:
RevertToCurrentSnapShot();
DiscardSnapshotHandler(CurrentSnapshot);
- 'Discard Current Snapshot and State', when the current snapshot has a Parent, in other words there are two or more snapshots, does this:
DiscardSnapshotHandler(CurrentSnapshot);
RevertToCurrentSnapShot();
This last behaviour is broken and unwanted and silly and inefficient.
Using the first bug example given above, which has two snaphots named '1 January' and 'Before 1 April Experiment', the call to DiscardSnapshotHandler(CurrentSnapshot) Discards 'Before 1 April Experiment', merging its changes (the 1st April experiment) with the changes stored in '1 January' (the work January/February/March).
'1 January' is now the Current snapshot.
Then the call to RevertToCurrentSnapShot() throws away the merged changes stored in '1 January', throwing away the experiment, BUT ALSO throwing away all of the work January/February/March, which is a severe problem.
(and also, why bother merging the changes if they are only going to be thrown away?)
'Discard Current Snapshot and State', when there are 2 or more snapshots, should function exactly the same as when there is only one snapshot, and call RevertToCurrentSnapShot() first, and DiscardSnapshotHandler() second.
That behaviour is useful and sensible.
3) The behaviour suggested above is useful, whereas the current 'Go back two snapshots' behaviour is mostly useless.
The suggested 'Revert to Last Snapshot then Discard Snapshot' function is useful in situations where the current state must be protected while an experimental configuration or change is done.
If the experiment is successful and should be retained, the user uses 'Discard Snapshot'.
If the experiment needs several iterations to get right, the user uses 'Revert to Snapshot' and tries again.
If the experiment is not successful and needs to be abandoned, the user uses 'Revert to Last Snapshot then Discard Snapshot' (='Delete Current Snapshot and State').
4) And ... VirtualBox developers: Thank you for the great software!
Edit: Also entered as a ticket: http://www.virtualbox.org/ticket/5182
.