[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