On 05/15/2012 03:28 AM, Chris Evich wrote: > On 05/11/2012 09:38 PM, tangchen wrote: >> 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? :) > > Yep, always happy to see extensions like this added. I've got a > laundry list of them myself, which I plan on working over the coming > months. > > I seem to remember reading at some point, someone wanted to split the > virsh_*() functions off into a virsh.py module. If you want to > tackle that, please feel free, but it's not urgent by any stretch. I'm > sure it'll come up again when the code/test split nears. I'm not > super particular about where the code lands, and libvirt_vm.py sounds > like a good place for now. I'm working on virt-v2v automation testing now, and I will integrate it into autotest later, you know virt-v2v supports libvirt and rhev output method(target) then I will reuse current libvirt-autotest as libvirt target, as Chris said, we need to re-factoring some modules such as libvirt_vm.py, for details, please see previous discussion with Lucas and Chris. However, it's okay for your patch before codes/framework re-factoring.
It's also fine If anyone want to re-factoring libvirt-autotest before me. > >>> 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. :) >> > > I'm still working on getting some lab boxes, but have it mostly > figured out. I'm excited to see all these new migration tests running! > _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
