At 2014-12-05 22:26:10, "Cole Robinson" <[email protected]> wrote: >On 12/03/2014 12:20 AM, Chen, Hanxiao wrote: >> >> >>> -----Original Message----- >>> From: Cole Robinson [mailto:[email protected]] >>> Sent: Tuesday, December 02, 2014 11:00 PM >>> To: Chen, Hanxiao/陈 晗霄; [email protected] >>> Subject: Re: [virt-tools-list] [virt-manager PATCH] test: fix a virt-clone >>> test >>> cases >>> >>> On 12/01/2014 10:39 PM, Chen Hanxiao wrote: >>>> remove '--force option' in test case virtclone0283 >>>> >>>> Signed-off-by: Chen Hanxiao <[email protected]> >>>> --- >>>> tests/clitest.py | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/tests/clitest.py b/tests/clitest.py >>>> index 6b3e45d..5c9d635 100644 >>>> --- a/tests/clitest.py >>>> +++ b/tests/clitest.py >>>> @@ -916,7 +916,7 @@ c.add_invalid("--original-xml %(CLONE_DISK_XML)s >>> --file %(NEWCLONEIMG1)s --file >>>> c.add_invalid("--original-xml %(CLONE_STORAGE_XML)s --file >>>> /tmp/clonevol") >>> # XML w/ managed storage, specify unmanaged path (should fail) >>>> c.add_invalid("--original-xml %(CLONE_NOEXIST_XML)s --file >>>> %(EXISTIMG1)s") >>> # XML w/ non-existent storage, WITHOUT --preserve >>>> c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file %(ROIMG)s >>> --file %(ROIMG)s --force") # XML w/ managed storage, specify RO image >>> without >>> preserve >>>> -c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file %(ROIMG)s >>> --file %(ROIMGNOEXIST)s --force") # XML w/ managed storage, specify RO non >>> existent >>>> +c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file %(ROIMG)s >>> --file %(ROIMGNOEXIST)s") # XML w/ managed storage, specify RO non existent >>>> >>>> >>>> >>>> >>> >>> Can you explain exactly what's wrong here? >>> >> >> testCLIvirtclone0283: >> >> When I run test suites, I got a NG case: >> ./virt-clone --debug --connect >> __virtinst_test__test:////home/virt-manager/tests/testdriver.xml,predictable >> -n clonetest --original-xml tests/cli-test-xml/clone-disk.xml --file >> /tmp/__virtinst_cli_clitest_rodir/cli_exist3ro.img --file >> /tmp/__virtinst_cli_clitest_rodir/idontexist.img --force >> >> In clone-disk.xml, we got 2 disk device and a CD-ROM to clone. >> For output, we specified two files, one of them not existed. >> >> --force will set force option. >> >> then we could see: >> DEBUG (diskbackend:149) Path '/tmp/__virtinst_cli_clitest_rodir' is target >> for pool '__virtinst_cli_clitest_rodir-3'. Creating volume >> 'cli_exist3ro.img'. >> [Wed, 03 Dec 2014 11:57:29 virt-clone 21637] DEBUG (cli:321) --force >> skipping error condition 'This will overwrite the existing path >> '/tmp/__virtinst_cli_clitest_rodir/cli_exist3ro.img'' >> [Wed, 03 Dec 2014 11:57:29 virt-clone 21637] WARNING (cli:322) This will >> overwrite the existing path >> '/tmp/__virtinst_cli_clitest_rodir/cli_exist3ro.img' >> [Wed, 03 Dec 2014 11:57:29 virt-clone 21637] DEBUG (cli:321) --force >> skipping error condition 'This will overwrite the existing path >> '/tmp/__virtinst_cli_clitest_rodir/cli_exist3ro.img'' >> [Wed, 03 Dec 2014 11:57:29 virt-clone 21637] WARNING (cli:322) This will >> overwrite the existing path >> '/tmp/__virtinst_cli_clitest_rodir/cli_exist3ro.img' >> >> without --force, this case will fail, as add_invalid expected. >> >> Does this case be designed to test this? >> >> As Giuseppe's comments, I think we should remove both '--force' option. >> >> diff --git a/tests/clitest.py b/tests/clitest.py >> index 6b3e45d..759b472 100644 >> --- a/tests/clitest.py >> +++ b/tests/clitest.py >> @@ -915,8 +915,8 @@ c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file >> virt-install --file %(EX >> c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file %(NEWCLONEIMG1)s >> --file %(NEWCLONEIMG2)s --fo >> c.add_invalid("--original-xml %(CLONE_STORAGE_XML)s --file /tmp/clonevol") >> # XML w/ managed storage, >> c.add_invalid("--original-xml %(CLONE_NOEXIST_XML)s --file %(EXISTIMG1)s") >> # XML w/ non-existent sto >> -c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file %(ROIMG)s --file >> %(ROIMG)s --force") # XML w >> -c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file %(ROIMG)s --file >> %(ROIMGNOEXIST)s --force") >> +c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file %(ROIMG)s --file >> %(ROIMG)s") # XML w/ manage >> +c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file %(ROIMG)s --file >> %(ROIMGNOEXIST)s") # XML w/ >> > >Change the second instance to c.add_valid, the test suite gives me: > >$ python setup.py test --only testCLIvirtclone0283 >... >RuntimeError: Error cloning diskimage /tmp/__virtinst_cli_exist1.img to >/tmp/__virtinst_cli_clitest_rodir/cli_exist3ro.img: [Errno 13] Permission >denied: '/tmp/__virtinst_cli_clitest_rodir/cli_exist3ro.img' > >Which is the error condition we are trying to test for (clone choking on >trying to copy to a read-only image) > >If I then remove --force, it then errors with: > >$ python setup.py test --only testCLIvirtclone0283 >... >[Fri, 05 Dec 2014 09:24:46 virt-clone 13894] ERROR (cli:263) This will >overwrite the existing path >'/tmp/__virtinst_cli_clitest_rodir/cli_exist3ro.img' (Use --force to override) > >Which shows that --force is required to test the read-only scenario. So I'm >still not sure what the point of your patch is, the test suite is running fine >for me at the moment. Are you seeing failures without your patch? > I use fedora20 with upstream libvirt.It gives me 'pass' with --force for these two cases.(282, 283) for a long time.
Thanks, - Chen
_______________________________________________ virt-tools-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/virt-tools-list
