On 11/30/2011 03:53 PM, Lucas Meneghel Rodrigues wrote: > On 11/30/2011 12:24 PM, Chris Evich wrote: >> Change login key name from _duration to _timeout for clarity, since other >> variables named _duration are used differently. There was a race condition >> on >> end_time where test could fail if final savevm_delay plus testing time, took >> longer than pre-set end_time. Modify test to loop through a fixed number of >> save/restore cycles instead of testing for a fixed time period. The duration >> of each cycle is compared to the second cycle. Any lengthening of cycle >> duration beyond a threshold causes test to fail. > > Ok, some comments, already went through most of them on irc: > > 1) The original purpose of the patch is to stop/save/load the vm many > times *while* the same is booting, so the original logic of the test is > right > 2) Perhaps we could make some timeouts smaller so more start/stop > cycles could happen > 3) I'd be fine with a fixed number of cycles, if we move the vm login > outside the loop. However, I think a variable number of cycles is smarter. > 4) I'm looking at the original code and I think there's no possible > way that the mentioned race condition could happen, as the termination > condition for while loops is check on the beginning of the loop, not at > the end.
Ok, I do see now where the race could happen :) Nevertheless, the fix I proposed is valid and simpler, so I'd go with it. _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
