Lucas, Thanks for the feedback. Some questions/comments inline (and for benefit of others)...
On 12/06/2011 11:48 PM, Lucas Meneghel Rodrigues wrote: ...cut... >> +def __virsh_state_file_generate(name, uri = ""): > > ^ In order to ease unittesting, in general in autotest we avoid double > underscore. Double underscores are OK only if there's any benefit in > mangling the name and making it inaccessible from outside the class, > which is usually none :) Ahh, gotcha, yep realized in hind-sight this would be a generally useful function (see below). I'll keep the underscore guideline in mind also, thanks. >> + """ >> + Internal function to generate absolute path to state file name >> + ... > ^ Here we could add some random string to the file name, to avoid > clashes of state files taken in different situations/running in parallel. I'm thinking we need a hash that's a little state-full than random(). Maybe VM name + first NIC mac address (or something like that). This needs some more thinking about, b/c it's potentially applicable independent of hypervisor type. VM's need host-side filenames that don't clash but can be derived outside any particular VM instance or host. >> +def virsh_save(name, uri = "", path = ""): >> + """ >> + Store state of VM into named file or temporary file if not specified. >> + ... Okay, I'll take master, toss all these patches back on to it. Fix up the things you mentioned, and re-submit. K? -- Chris Evich, RHCA, RHCE, RHCDS, RHCSS Quality Assurance Engineer e-mail: cevich + `@' + redhat.com o: 1-888-RED-HAT1 x44214 -- Chris Evich, RHCA, RHCE, RHCDS, RHCSS Quality Assurance Engineer e-mail: cevich + `@' + redhat.com o: 1-888-RED-HAT1 x44214 _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
