On Wed, Mar 06, 2013 at 01:51:48AM -0300, Lucas Meneghel Rodrigues wrote: > On Mon, Mar 4, 2013 at 8:29 AM, Satheesh Rajendran > <[email protected]> wrote: > > Hi Lucas, > > > > I had sent the reworked V2 patch aswell which includes the switch in runner, > > Could you please validate both the options based upon the discussion here > > and push to the appropriate to next. > > I did take a look at this patchset for inclusion, and there are things > that still need to be addressed: > > 1) The output of "hostname" is good only if the host has a fully > qualified domain name. If it's a laptop installed on a domestic > network, this will fail and gluster will say that 'host blabla is not > a friend'. Therefore, if the hostname can't be resolved, this must > fall back to IP.
Agreed, I would check on this. > 2) There is the use of the commands API throughout the module. I'd > prefer this usage is replaced with utils.system, or utils.run, > whatever is more convenient. Sure, I will replace. > 3) Indeed, you don't need to create a runner switch. Thanks, I would revert in the next version. > 4) With the introduction of variants in guest_hw.cfg, you necessarily > have to update a lot of test.cfg files inside virt-tests to say things > like 'no glusterfs_support', otherwise all control files and test runs > will create more tests than they should. Sure, Will add the details in tests.cfg. I can update the wiki and give the reference to the wiki in the test.cfg. > 5) With all enabled, qemu-img will still try to create an image, which > is incorrect behavior. > > 01:37:26 ERROR| Could not create image, failed with error > message:Command </home/lmr/Code/qemu/qemu-img create -f qcow2 > gluster://192.168.2.5:0/test-vol/f18-64.qcow2 10G> failed, rc=1, > Command returned non-zero exit status > * Command: > /home/lmr/Code/qemu/qemu-img create -f qcow2 gluster://192.168.2.5:0 > /test-vol/f18-64.qcow2 10G > Exit status: 1 > Duration: 0.00223588943481 > > stderr: > qemu-img: Unknown protocol 'gluster://192.168.2.5:0/test-vol/f18-64.qcow2' > > I'd say implementing glusterfs as a new image type, that does not use > qemu img to create it seems better than the approach you're taking. > The following infrastructure is required to run this test Qemu- 1.3, 03Dec2012 GlusterFS-3.4 Libvirt-1.0.1, 15Dec2012 (This is not required for this patch, would be helpful for the libvirt support in future) Agreed, I should have added this details in the first place in a wiki, that would have cleared the confusions, I will update it the wiki. > 6) Glusterfs usage has to be covered in the wiki Sure. > 7) Some example config files for the qemu test would also help. This might not be required as it is going be just a enabler, then all the tests can run on the image created on gluster volume. > > So I'd say there are a lot of things to be fixed. Let's get to work on them. > > Thanks! > Thanks for the detailed review, Would send the updated patch soon. Regards, -Satheesh. _______________________________________________ Virt-test-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/virt-test-devel
