Hi, Chris: Thanks for all these comments. :)
> tangchen, > > Thanks a TON for all these great tests, overall it's very thorough and > extensive. I'd really like to see some of these tests in action, but didn't > want to spend lots of time setting it up w/o providing some initial feedback: > > There are a handful of whitespace errors: > client/tests/libvirt/tests/virsh_migrate.py line 45 > client/tests/libvirt/tests/virsh_migrate.py line 263 > client/virt/subtests.cfg.sample line 586 > client/virt/subtests.cfg.sample around line 592 OK, I'll fix them. > > do_error_migration() > This is a good catch, we need to do negative testing like this. However, > implementing it inside a test makes me a little uncomfortable. It may be > that other test modules would like to do migration error testing but aren't > able to because the code is locked up in this test. > > Your comment says the migration functions are hiding some error conditions, > which reads to me like a deficiency in the core code. Could we fix > vm.migrate() and/or virsh_migrate() to support error testing? Maybe a keyword > parameter with a default could be added to one or both to support it? > True. I thought about this before. My original idea was handling no error in vm.migrate(), but this was tough. As you said, a keyword parameter with a default is a quit good idea. I'm glad to finish it. Leave it to me. :) > do_migration() > Thanks for changing "dly" to "delay", it's much clearer. > > #FIXME: vm.verify_kernel_crash() > I'm pretty sure we fixed this by now, but I neglected to update this test. > I'll confirm by simple test (messing up root= parameter in grub). > > # For safety reasons, we'd better back up xmlfile. > This is a fantastic idea! What do you think about adding this as a method of > the vm class, so we can use it elsewhere also? > Not only back-up-xml function, but alse some other functions (such as rename-a-vm, which is under developping) may be useful in all libvirt tests, especially in error situations. And IMO, just like libvirtd_start/stop functions, we can put them together in libvirt_vm.py. What do you think? :) > If any of the above fixes/changes you don't have time/resources for, just let > me know and I'll take care of 'em. Overall it looks really good and I'll be > happy to see this applied. > It's no problem, I'll finish them. > I'll start working up a pair of lab boxes to run this code on. Depending on > how many "challenges" I run into doing that, it could be late next week > before I get to run any of these cool new tests. But I'll work on it and let > you know if I have more feedback. > > Thanks again! > Thanks. :) -- Best Regards, Tang chen _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
