[pve-devel] [PATCH qemu-server 1/2] add tests for qemu_img_convert

Dominik Csapak d.csapak at proxmox.com
Thu Oct 17 12:09:17 CEST 2019


On 10/17/19 9:26 AM, Thomas Lamprecht wrote:
> On 10/16/19 11:27 AM, Dominik Csapak wrote:
>> storage config is motly copied from the config2command tests
>>
> 
> some general info about what and *how* you test would be great here.
> 
> looks OK in general, and thanks for more test, some comments inline
> 
>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
>> ---
>>   test/Makefile                      |   5 +-
>>   test/run_qemu_img_convert_tests.pl | 186 +++++++++++++++++++++++++++++
>>   2 files changed, 190 insertions(+), 1 deletion(-)
>>   create mode 100755 test/run_qemu_img_convert_tests.pl
>>
>> diff --git a/test/Makefile b/test/Makefile
>> index 902d9e3..d88cbd2 100644
>> --- a/test/Makefile
>> +++ b/test/Makefile
>> @@ -1,6 +1,6 @@
>>   all: test
>>   
>> -test: test_snapshot test_ovf test_cfg_to_cmd test_pci_addr_conflicts
>> +test: test_snapshot test_ovf test_cfg_to_cmd test_pci_addr_conflicts test_qemu_img_convert
>>   
>>   test_snapshot: run_snapshot_tests.pl
>>   	./run_snapshot_tests.pl
>> @@ -12,5 +12,8 @@ test_ovf: run_ovf_tests.pl
>>   test_cfg_to_cmd: run_config2command_tests.pl cfg2cmd/*.conf
>>   	perl -I../ ./run_config2command_tests.pl
>>   
>> +test_qemu_img_convert: run_qemu_img_convert_tests.pl
>> +	perl -I../ ./run_qemu_img_convert_tests.pl
>> +
>>   test_pci_addr_conflicts: run_pci_addr_checks.pl
>>   	./run_pci_addr_checks.pl
>> diff --git a/test/run_qemu_img_convert_tests.pl b/test/run_qemu_img_convert_tests.pl
>> new file mode 100755
>> index 0000000..fafb58a
>> --- /dev/null
>> +++ b/test/run_qemu_img_convert_tests.pl
>> @@ -0,0 +1,186 @@
>> +#!/usr/bin/env perl
> 
> besides the trailing whitespace error I'd like to keep this in sync with
> the other tests and use "/usr/bin/perl" as #!

oops, my editor automatically used inserted /usr/bin/env perl ^^

> 
>> +
>> +use strict;
>> +use warnings;
>> +
>> +use lib qw(..);
>> +
>> +use Test::More;
>> +use Test::MockModule;
>> +
>> +use PVE::QemuServer;
>> +
>> +my $vmid = 8006;
>> +my $storage_config = {
>> +    ids => {
>> +	local => {
>> +	    content => {
>> +		images => 1,
>> +	    },
>> +	    path => "/var/lib/vz",
>> +	    type => "dir",
>> +	    shared => 0,
>> +	},
>> +	"rbd-store" => {
>> +	    monhost => "127.0.0.42,127.0.0.21,::1",
>> +	    content => {
>> +		images => 1
>> +	    },
>> +	    type => "rbd",
>> +	    pool => "cpool",
>> +	    username => "admin",
>> +	    shared => 1
>> +	},
>> +	"local-lvm" => {
>> +	    vgname => "pve",
>> +	    bwlimit => "restore=1024",
>> +	    type => "lvmthin",
>> +	    thinpool => "data",
>> +	    content => {
>> +		images => 1,
>> +	    }
>> +	},
>> +	"zfs-over-iscsi" => {
>> +	    type => "zfs",
>> +	    iscsiprovider => "LIO",
>> +	    lio_tpg => "tpg1",
>> +	    portal => "127.0.0.1",
>> +	    target => "iqn.2019-10.org.test:foobar",
>> +	    pool => "tank",
>> +	}
>> +    }
>> +};
>> +
>> +my $tests = {
>> +    qcow2raw => [
>> +	"local:$vmid/vm-$vmid-disk-0.qcow2", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0, [
>> +	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "qcow2", "-O", "raw",
>> +	    "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.qcow2", "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw"
>> +	]
>> +    ],
> 
> can e have an explicit separation of command and expected result already in
> the test data structure representation?
> 
> something like:
> 
> qcow2raw => {
>    cmd => [],
>    expected => ...,
> }

sure but i would it do like this:

[
  	{
		name => 'testname',
		test => [...],
		expected => [...],
	},
	...
]

that way we have control about the order of the tests...

> 
>> +    raw2qcow2 => [
>> +	"local:$vmid/vm-$vmid-disk-0.raw", "local:$vmid/vm-$vmid-disk-0.qcow2", 1024*10, undef, 0, [
>> +	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "-O", "qcow2",
>> +	    "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw", "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.qcow2"
>> +	]
>> +    ],
>> +    local2rbd => [
>> +	"local:$vmid/vm-$vmid-disk-0.raw", "rbd-store:vm-$vmid-disk-0", 1024*10, undef, 0, [
>> +	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "-O", "raw",
>> +	    "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw", "rbd:cpool/vm-$vmid-disk-0:mon_host=127.0.0.42;127.0.0.21;[\\:\\:1]:auth_supported=none"
>> +	]
>> +    ],
>> +    rbd2local => [
>> +	"rbd-store:vm-$vmid-disk-0", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0, [
>> +	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "-O", "raw",
>> +	    "rbd:cpool/vm-$vmid-disk-0:mon_host=127.0.0.42;127.0.0.21;[\\:\\:1]:auth_supported=none", "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw"
>> +	]
>> +    ],
>> +    local2zos => [
>> +	"local:$vmid/vm-$vmid-disk-0.raw", "zfs-over-iscsi:vm-$vmid-disk-0", 1024*10, undef, 0, [
>> +	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "--target-image-opts",
>> +	    "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw",
>> +	    "file.driver=iscsi,file.transport=tcp,file.initiator-name=foobar,file.portal=127.0.0.1,file.target=iqn.2019-10.org.test:foobar,file.lun=1,driver=raw"
>> +	]
>> +    ],
>> +    zos2local => [
>> +	"zfs-over-iscsi:vm-$vmid-disk-0", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0, [
>> +	    "/usr/bin/qemu-img", "convert", "-p", "-n", "--image-opts", "-O", "raw",
>> +	    "file.driver=iscsi,file.transport=tcp,file.initiator-name=foobar,file.portal=127.0.0.1,file.target=iqn.2019-10.org.test:foobar,file.lun=1,driver=raw",
>> +	    "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw",
>> +	]
>> +    ],
>> +    zos2rbd => [
>> +	"zfs-over-iscsi:vm-$vmid-disk-0", "rbd-store:vm-$vmid-disk-0", 1024*10, undef, 0, [
>> +	    "/usr/bin/qemu-img", "convert", "-p", "-n", "--image-opts", "-O", "raw",
>> +	    "file.driver=iscsi,file.transport=tcp,file.initiator-name=foobar,file.portal=127.0.0.1,file.target=iqn.2019-10.org.test:foobar,file.lun=1,driver=raw",
>> +	    "rbd:cpool/vm-$vmid-disk-0:mon_host=127.0.0.42;127.0.0.21;[\\:\\:1]:auth_supported=none"
>> +	]
>> +    ],
>> +    rbd2zos => [
>> +	"rbd-store:vm-$vmid-disk-0", "zfs-over-iscsi:vm-$vmid-disk-0", 1024*10, undef, 0, [
>> +	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "--target-image-opts",
>> +	    "rbd:cpool/vm-$vmid-disk-0:mon_host=127.0.0.42;127.0.0.21;[\\:\\:1]:auth_supported=none",
>> +	    "file.driver=iscsi,file.transport=tcp,file.initiator-name=foobar,file.portal=127.0.0.1,file.target=iqn.2019-10.org.test:foobar,file.lun=1,driver=raw",
>> +	]
>> +    ],
>> +    local2lvmthin => [
>> +	"local:$vmid/vm-$vmid-disk-0.raw", "local-lvm:vm-$vmid-disk-0", 1024*10, undef, 0, [
>> +	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "-O", "raw",
>> +	    "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw",
>> +	    "/dev/pve/vm-$vmid-disk-0",
>> +	]
>> +    ],
>> +    lvmthin2local => [
>> +	"local-lvm:vm-$vmid-disk-0", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0, [
>> +	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "-O", "raw",
>> +	    "/dev/pve/vm-$vmid-disk-0",
>> +	    "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw",
>> +	]
>> +    ],
>> +    zeroinit => [
>> +	"local-lvm:vm-$vmid-disk-0", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 1, [
>> +	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "-O", "raw",
>> +	    "/dev/pve/vm-$vmid-disk-0",
>> +	    "zeroinit:/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw",
>> +	]
>> +    ],
>> +    notexistingstorage => [
>> +	"local-lvm:vm-$vmid-disk-0", "not-existing:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 1,
>> +	"storage 'not-existing' does not exists\n",
>> +    ],
>> +};
>> +
>> +my $command;
>> +
>> +my $storage_module = Test::MockModule->new("PVE::Storage");
>> +$storage_module->mock(
>> +    config => sub {
>> +	return $storage_config;
>> +    },
>> +    activate_volumes => sub {
>> +	return 1;
>> +    }
>> +);
>> +
>> +my $lio_module = Test::MockModule->new("PVE::Storage::LunCmd::LIO");
>> +$lio_module->mock(
>> +    run_lun_command => sub {
>> +	return 1;
>> +    }
>> +);
>> +
>> +# we use the exported run_command so we have to mock it there
>> +my $zfsplugin_module = Test::MockModule->new("PVE::Storage::ZFSPlugin");
>> +$zfsplugin_module->mock(
>> +    run_command => sub {
>> +	return 1;
>> +    }
>> +);
>> +
>> +# we use the exported run_command so we have to mock it there
>> +my $qemu_server_module = Test::MockModule->new("PVE::QemuServer");
>> +$qemu_server_module->mock(
>> +    run_command => sub {
>> +	$command = shift;
>> +    },
>> +    get_initiator_name => sub {
>> +	return "foobar";
>> +    }
>> +);
>> +
>> +foreach my $name (sort keys %$tests) {
>> +    my $res = $tests->{$name}->[-1]; # last element
> 
> /if/ we would have res and cmd mixed, then I'd at least do
> my $res = pop @{$tests->{$name}};
> 
> so that it does not gets expanded in the qemu_img_convert call below..
> 

ok noted

>> +    eval { PVE::QemuServer::qemu_img_convert(@{$tests->{$name}}) };
>> +    if (my $err = $@) {
>> +	is ($err, $res, $name);
>> +    } elsif (defined($command)) {
>> +	is_deeply($command, $res, $name);
>> +	$command = undef;
>> +    } else {
>> +	fail($name);
>> +	note("no command")
>> +    }
>> +}
>> +
>> +done_testing();
>>
> 





More information about the pve-devel mailing list