(Sorry, I missed these while I was off remodeling) On 05/21/2018 12:46 PM, Daniel P. Berrangé wrote: Reviewed-by: Laine Stump <laine(a)laine.org&gt; At first I was wondering how these macs ever got in. Then I remembered that libvirt used to not check for multicast addresses.

On 05/21/2018 12:46 PM, Daniel P. Berrangé wrote: I'm curious what you were doing that you ended up setting this.  Running tests against qemu:///session? Reviewed-by: Laine Stump <laine(a)laine.org&gt;

The semantics of the "physical" field were fixed to not duplicate the info of the "allocation" field. The use of block_peek on non-raw volumes now raises an error since we cannot peek inside volumes. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- scripts/domain/121-block-info.t | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/scripts/domain/121-block-info.t b/scripts/domain/121-block-info.t index 6fa7780..bfff48c 100644 --- a/scripts/domain/121-block-info.t +++ b/scripts/domain/121-block-info.t @@ -99,7 +99,7 @@ $xml = $dom->get_xml_description(); is($dom->get_block_info($dst2,0)->{capacity}, 1024*1024*50, "Get disk capacity info"); is($dom->get_block_info($dst2,0)->{allocation}, 1024*1024, "Get disk allocation info"); -is($dom->get_block_info($dst2,0)->{physical}, 1024*1024, "Get disk physical info"); +is($dom->get_block_info($dst2,0)->{physical}, 1024*1024*50, "Get disk physical info"); is($dom->get_block_info($dst,0)->{capacity}, 1024*1024*50, "Get disk capacity info"); @@ -124,8 +124,7 @@ my $date = "test"; system("echo $date > $path"); is($dom->block_peek($path,0,4,0), $date, "Get date from raw image"); -#qcow2 file start with hexadecimal:0x51 0x46 0x49 (ASCII: "QFI") -is($dom->block_peek($path3,0,3,0), "QFI", "Get date from qcow2 image"); +dies_ok(sub { $dom->block_peek($path3,0,3,0) }, "Get date from qcow2 image"); lives_ok(sub { $vol->delete(0) }, "deleted volume"); -- 2.17.0

On 05/21/2018 12:46 PM, Daniel P. Berrangé wrote: Reviewed-by: Laine Stump <laine(a)laine.org&gt; So you've removed to calls to ok(), one to ok_volume(), and one to lives_ok(), so that checks out (and the test succeeds :-) For each format, there are 9 tests, so that checks out too. (Sorry for being verbose about such a simple thing - it's just that I hadn't even noticed this number before today, and am just now seeing where it comes from :-)

On 05/21/2018 12:46 PM, Daniel P. Berrangé wrote: Reviewed-by: Laine Stump <laine(a)laine.org&gt;

Libvirt may report two seclabels, one for SELinux and one for DAC. Make sure we grab the right one from the XML. Also fix the number of tests. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- scripts/selinux/100-static-relabel-no.t | 4 ++-- scripts/selinux/110-static-relabel-yes.t | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/selinux/100-static-relabel-no.t b/scripts/selinux/100-static-relabel-no.t index 8d9fda8..ab52534 100644 --- a/scripts/selinux/100-static-relabel-no.t +++ b/scripts/selinux/100-static-relabel-no.t @@ -62,9 +62,9 @@ SKIP: { my $dom; ok_domain(sub { $dom = $conn->create_domain($xml) }, "created transient domain object"); - my $domainlabel = xpath($dom, "string(/domain/seclabel/label)"); + my $domainlabel = xpath($dom, "string(/domain/seclabel[\@model='selinux']/label)"); diag "domainlabel $domainlabel"; - my $imagelabel = xpath($dom, "string(/domain/seclabel/imagelabel)"); + my $imagelabel = xpath($dom, "string(/domain/seclabel[\@model='selinux']/imagelabel)"); diag "imagelabel $imagelabel"; is($origdomainlabel, $domainlabel, "static label is $domainlabel"); diff --git a/scripts/selinux/110-static-relabel-yes.t b/scripts/selinux/110-static-relabel-yes.t index 76781ca..18350cf 100644 --- a/scripts/selinux/110-static-relabel-yes.t +++ b/scripts/selinux/110-static-relabel-yes.t @@ -28,7 +28,7 @@ and files can be relabelled use strict; use warnings; -use Test::More tests => 5; +use Test::More tests => 6; use Sys::Virt::TCK; use Sys::Virt::TCK::SELinux; @@ -62,9 +62,9 @@ SKIP: { ok_domain(sub { $dom = $conn->create_domain($xml) }, "created transient domain object"); diag $dom->get_xml_description(); - my $domainlabel = xpath($dom, "string(/domain/seclabel/label)"); + my $domainlabel = xpath($dom, "string(/domain/seclabel[\@model='selinux']/label)"); diag "domainlabel $domainlabel"; - my $imagelabel = xpath($dom, "string(/domain/seclabel/imagelabel)"); + my $imagelabel = xpath($dom, "string(/domain/seclabel[\@model='selinux']/imagelabel)"); diag "imagelabel $imagelabel"; my $imagetype = selinux_get_type($imagelabel); my $imagemcs = selinux_get_mcs($imagelabel); -- 2.17.0

The ability to use qcow legacy encryption is disabled with QEMU for a long time. Switch to using luks encryption, although this is not yet fully wired up in libvirt so the tests still (temporarily) fail. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- lib/Sys/Virt/TCK/DomainBuilder.pm | 4 ++-- lib/Sys/Virt/TCK/StorageVolBuilder.pm | 18 +++++++++++++----- scripts/qemu/100-disk-encryption.t | 2 ++ 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/lib/Sys/Virt/TCK/DomainBuilder.pm b/lib/Sys/Virt/TCK/DomainBuilder.pm index 83cea15..b6adbd3 100644 --- a/lib/Sys/Virt/TCK/DomainBuilder.pm +++ b/lib/Sys/Virt/TCK/DomainBuilder.pm @@ -412,8 +412,8 @@ sub as_xml { $w->emptyTag("target", dev => $disk->{dst}, $disk->{bus} ? (bus => $disk->{bus}) : ()); - if ($disk->{secret}) { - $w->startTag("encryption", format => "qcow"); + if ($disk->{encformat}) { + $w->startTag("encryption", format => $self->{encryption_format}); $w->emptyTag("secret", type => "passphrase", uuid => $disk->{secret}); $w->endTag("encryption"); } diff --git a/lib/Sys/Virt/TCK/StorageVolBuilder.pm b/lib/Sys/Virt/TCK/StorageVolBuilder.pm index 7208f75..ad950ed 100644 --- a/lib/Sys/Virt/TCK/StorageVolBuilder.pm +++ b/lib/Sys/Virt/TCK/StorageVolBuilder.pm @@ -60,6 +60,14 @@ sub format { return $self; } +sub encryption_format { + my $self = shift; + + $self->{encformat} = shift; + + return $self; +} + sub secret { my $self = shift; @@ -95,13 +103,13 @@ sub as_xml { $w->dataElement("capacity", $self->{capacity}); $w->dataElement("allocation", $self->{allocation}); - if ($self->{format} || $self->{secret}) { + if ($self->{format} || $self->{encformat}) { $w->startTag("target"); if ($self->{format}) { $w->emptyTag("format", type => $self->{format}); } - if ($self->{secret}) { - $w->startTag("encryption", format => "qcow"); + if ($self->{encformat}) { + $w->startTag("encryption", format => $self->{encformat}); $w->emptyTag("secret", type => "passphrase", uuid => $self->{secret}); $w->endTag("encryption"); } @@ -114,8 +122,8 @@ sub as_xml { if ($self->{backingFormat}) { $w->emptyTag("format", type => $self->{backingFormat}); } - if ($self->{secret}) { - $w->startTag("encryption", format => "qcow"); + if ($self->{encformat}) { + $w->startTag("encryption", format => $self->{encformat}); $w->emptyTag("secret", type => "passphrase", uuid => $self->{secret}); $w->endTag("encryption"); } diff --git a/scripts/qemu/100-disk-encryption.t b/scripts/qemu/100-disk-encryption.t index 8a0fd5e..3b1651a 100644 --- a/scripts/qemu/100-disk-encryption.t +++ b/scripts/qemu/100-disk-encryption.t @@ -76,6 +76,7 @@ lives_ok(sub { $pool = $conn->create_storage_pool($poolXML) }, "pool created"); my $volXML = Sys::Virt::TCK::StorageVolBuilder->new(name => "demo.qcow2") ->capacity(1024*1024*1024) ->format("qcow2") + ->encryption_format("luks") ->secret($secretUUID) ->as_xml(); @@ -86,6 +87,7 @@ lives_ok(sub { $vol = $pool->create_volume($volXML) }, "volume created"); my $xml = $tck->generic_domain(name => "tck") ->disk(format => { name => "qemu", type => "qcow2" }, + encryption_format => "luks", secret => $secretUUID, type => "file", src => $disk, -- 2.17.0

On 06/01/2018 09:06 PM, Laine Stump wrote: Actually, when I did a complete build/install and tested this, I found that libvirt-tck's self-tests are now failing because the <encryption format='qcow'> element is no longer added to the <disk> in t/070-domain-builder.t and t/100-storage-vol-builder.t. If you want to squash that change into this patch, that's fine, otherwise you can make a separate patch, or if you don't want to I can.

Libvirt support for luks-inside-qcow2 is not quite ready yet. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- scripts/qemu/100-disk-encryption.t | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/qemu/100-disk-encryption.t b/scripts/qemu/100-disk-encryption.t index 3b1651a..32e2adf 100644 --- a/scripts/qemu/100-disk-encryption.t +++ b/scripts/qemu/100-disk-encryption.t @@ -42,6 +42,7 @@ END { $tck->cleanup if $tck; } SKIP: { skip "Only relevant to QEMU driver", 8 unless $conn->get_type() eq "QEMU"; + skip "Libvirt LUKS support isn't ready", 8; my $dir = $tck->bucket_dir("300-disk-encryption"); my $disk = catfile($dir, "demo.qcow2"); -- 2.17.0

PCI device hotunplug requires a running guest OS that cooperates with the ACPI unplug request. We must thus boot a real guest and wait for it to be ready before running the tests. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- scripts/domain/200-disk-hotplug.t | 4 +++- scripts/domain/205-disk-hotplug-ordering.t | 5 ++++- scripts/domain/210-nic-hotplug.t | 5 ++++- scripts/domain/215-nic-hotplug-many.t | 5 ++++- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/scripts/domain/200-disk-hotplug.t b/scripts/domain/200-disk-hotplug.t index 4c54b6b..87c0e53 100644 --- a/scripts/domain/200-disk-hotplug.t +++ b/scripts/domain/200-disk-hotplug.t @@ -41,12 +41,14 @@ END { } -my $xml = $tck->generic_domain(name => "tck")->as_xml; +my $xml = $tck->generic_domain(name => "tck", fullos => 1)->as_xml; diag "Creating a new transient domain"; my $dom; ok_domain(sub { $dom = $conn->create_domain($xml) }, "created transient domain object"); +diag "Waiting 30 seconds for guest to finish booting"; +sleep(30); my $path = $tck->create_sparse_disk("200-disk-hotplug", "extra.img", 100); diff --git a/scripts/domain/205-disk-hotplug-ordering.t b/scripts/domain/205-disk-hotplug-ordering.t index bc4990f..b76cd54 100644 --- a/scripts/domain/205-disk-hotplug-ordering.t +++ b/scripts/domain/205-disk-hotplug-ordering.t @@ -41,12 +41,15 @@ END { } -my $xml = $tck->generic_domain(name => "tck")->as_xml; +my $xml = $tck->generic_domain(name => "tck", fullos => 1)->as_xml; diag "Creating a new transient domain"; my $dom; ok_domain(sub { $dom = $conn->create_domain($xml) }, "created transient domain object"); +diag "Waiting 30 seconds for guest to finish booting"; +sleep(30); + my $supported = 1; foreach my $dev (qw/vdb sdb/) { my $path = $tck->create_sparse_disk("200-disk-hotplug", "extra-$dev.img", 100); diff --git a/scripts/domain/210-nic-hotplug.t b/scripts/domain/210-nic-hotplug.t index ac9048e..7106fd0 100644 --- a/scripts/domain/210-nic-hotplug.t +++ b/scripts/domain/210-nic-hotplug.t @@ -41,12 +41,15 @@ END { } -my $xml = $tck->generic_domain(name => "tck")->as_xml; +my $xml = $tck->generic_domain(name => "tck", fullos => 1)->as_xml; diag "Creating a new transient domain"; my $dom; ok_domain(sub { $dom = $conn->create_domain($xml) }, "created transient domain object"); +diag "Waiting 30 seconds for guest to finish booting"; +sleep(30); + my $mac = "00:11:22:33:44:55"; my $model = "virtio"; diff --git a/scripts/domain/215-nic-hotplug-many.t b/scripts/domain/215-nic-hotplug-many.t index eaa282a..558e6e7 100644 --- a/scripts/domain/215-nic-hotplug-many.t +++ b/scripts/domain/215-nic-hotplug-many.t @@ -41,12 +41,15 @@ END { } -my $xml = $tck->generic_domain(name => "tck")->as_xml; +my $xml = $tck->generic_domain(name => "tck", fullos => 1)->as_xml; diag "Creating a new transient domain"; my $dom; ok_domain(sub { $dom = $conn->create_domain($xml) }, "created transient domain object"); +diag "Waiting 30 seconds for guest to finish booting"; +sleep(30); + my $mac1 = "02:11:22:33:44:55"; my $mac2 = "02:12:22:33:44:55"; my $mac3 = "02:13:22:33:44:55"; -- 2.17.0

It is reasonable to run TCK against a manually started libvirtd, so we should not abort the entire test suite. Just mark the hook scripts as skipped. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- scripts/hooks/051-daemon-hook.t | 2 +- scripts/hooks/052-domain-hook.t | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/scripts/hooks/051-daemon-hook.t b/scripts/hooks/051-daemon-hook.t index aa7668b..82cedee 100644 --- a/scripts/hooks/051-daemon-hook.t +++ b/scripts/hooks/051-daemon-hook.t @@ -51,7 +51,7 @@ SKIP: { log_name => '/tmp/daemon.log'); $hook->libvirtd_status(); - BAIL_OUT "libvirtd is not running, Exit..." + skip "libvirtd is not running, Exit...", 12 if ($hook->{libvirtd_status} eq 'stopped'); eval { $hook->prepare(); }; diff --git a/scripts/hooks/052-domain-hook.t b/scripts/hooks/052-domain-hook.t index 07e4b23..c3a0b3f 100644 --- a/scripts/hooks/052-domain-hook.t +++ b/scripts/hooks/052-domain-hook.t @@ -49,17 +49,21 @@ SKIP: { skip "NOT using QEMU/LXC driver", 12 unless $uri eq "qemu:///system" or $uri eq "lxc:///"; + my $hook_type = $uri eq "qemu:///system" ? 'qemu' : 'lxc'; + + my $hook = Sys::Virt::TCK::Hooks->new(type => $hook_type, + conf_dir => '/etc/libvirt/hooks', + expect_result => 0); + $hook->libvirtd_status(); + skip "libvirtd is not running, Exit...", 12 + if ($hook->{libvirtd_status} eq 'stopped'); + my $xml = $tck->generic_domain(name => "tck")->as_xml; diag "Creating a new persistent domain"; my $dom; ok_domain(sub { $dom = $conn->define_domain($xml) }, "created persistent domain object"); - my $hook_type = $uri eq "qemu:///system" ? 'qemu' : 'lxc'; - - my $hook = Sys::Virt::TCK::Hooks->new(type => $hook_type, - conf_dir => '/etc/libvirt/hooks', - expect_result => 0); eval { $hook->prepare(); }; BAIL_OUT "failed to setup hooks testing ENV: $@" if $@; -- 2.17.0

The referenced QEMU bug is long since fixed Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- scripts/domain/102-broken-save-restore.t | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/domain/102-broken-save-restore.t b/scripts/domain/102-broken-save-restore.t index 4a05705..fea93f3 100644 --- a/scripts/domain/102-broken-save-restore.t +++ b/scripts/domain/102-broken-save-restore.t @@ -28,8 +28,7 @@ transient domains to/from a file. use strict; use warnings; -use Test::More skip_all => "Until RHBZ 518032 is fixed"; -#use Test::More tests => 5; +use Test::More tests => 5; use Sys::Virt::TCK; use Test::Exception; -- 2.17.0

Perl has started to forbid code from declaring "my $_", because $_ is implicitly always a local scope. Rewrite the code to avoid tickling this warning. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- lib/Sys/Virt/TCK/Hooks.pm | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/Sys/Virt/TCK/Hooks.pm b/lib/Sys/Virt/TCK/Hooks.pm index e7a8f76..7d35072 100644 --- a/lib/Sys/Virt/TCK/Hooks.pm +++ b/lib/Sys/Virt/TCK/Hooks.pm @@ -71,11 +71,10 @@ sub expect_result { sub libvirtd_status { my $self = shift; my $status = `service libvirtd status`; - my $_ = $status; - if (/stopped|unused|inactive/) { + if ($status =~ /stopped|unused|inactive/) { $self->{libvirtd_status} = 'stopped'; - } elsif (/running|active/) { + } elsif ($status =~ /running|active/) { $self->{libvirtd_status} = 'running'; } -- 2.17.0

One of the tests checks the ability to boot multiple guests at once, and it is using the same (empty) disk image for all. Modern QEMU rejects this sharing, but it is harmless in context of this test case, so we mark the disk as shareable to allow it. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- lib/Sys/Virt/TCK.pm | 6 +++++- lib/Sys/Virt/TCK/DomainBuilder.pm | 3 +++ scripts/domain/081-unique-id-create.t | 4 ++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm index e7ff71b..ac9c125 100644 --- a/lib/Sys/Virt/TCK.pm +++ b/lib/Sys/Virt/TCK.pm @@ -782,6 +782,7 @@ sub generic_machine_domain { my $caps = exists $params{caps} ? $params{caps} : die "caps parameter is required"; my $ostype = exists $params{ostype} ? $params{ostype} : "hvm"; my $fullos = exists $params{fullos} ? $params{fullos} : 0; + my $shareddisk = exists $params{shareddisk} ? $params{shareddisk} : 0; my $filterref = exists $params{filterref} ? $params{filterref} : undef; my %filterparams = exists $params{filterparams} ? %{$params{filterparams}} : (); @@ -857,7 +858,8 @@ sub generic_machine_domain { $b->disk(src => $config{root}, dst => $config{dev}, - type => "file"); + type => "file", + shareable => $shareddisk); return $b; } } @@ -915,6 +917,7 @@ sub generic_domain { my $ostype = exists $params{ostype} ? $params{ostype} : "hvm"; my $fullos = exists $params{fullos} ? $params{fullos} : 0; my $netmode = exists $params{netmode} ? $params{netmode} : undef; + my $shareddisk = exists $params{shareddisk} ? $params{shareddisk} : 0; my $filterref = exists $params{filterref} ? $params{filterref} : undef; my %filterparams = exists $params{filterparams} ? %{$params{filterparams}} : (); @@ -936,6 +939,7 @@ sub generic_domain { $b = $self->generic_machine_domain(name => $name, caps => $caps, ostype => $ostype, + shareddisk => $shareddisk, fullos => $fullos, filterref => $filterref, filterparams => \%filterparams); diff --git a/lib/Sys/Virt/TCK/DomainBuilder.pm b/lib/Sys/Virt/TCK/DomainBuilder.pm index b6adbd3..5ca2a07 100644 --- a/lib/Sys/Virt/TCK/DomainBuilder.pm +++ b/lib/Sys/Virt/TCK/DomainBuilder.pm @@ -409,6 +409,9 @@ sub as_xml { $w->emptyTag("source", file => $disk->{src}); } + if ($disk->{shareable}) { + $w->emptyTag("shareable"); + } $w->emptyTag("target", dev => $disk->{dst}, $disk->{bus} ? (bus => $disk->{bus}) : ()); diff --git a/scripts/domain/081-unique-id-create.t b/scripts/domain/081-unique-id-create.t index 07143a5..0d53599 100644 --- a/scripts/domain/081-unique-id-create.t +++ b/scripts/domain/081-unique-id-create.t @@ -54,13 +54,13 @@ my $uuid1 = "11111111-1111-1111-1111-111111111111"; my $uuid2 = "22222222-1111-1111-1111-111111111111"; # The initial config -my $xml = $tck->generic_domain(name => $name1)->uuid($uuid1)->as_xml; +my $xml = $tck->generic_domain(name => $name1, shareddisk => 1)->uuid($uuid1)->as_xml; # One with a different UUID, matching name my $xml_diffuuid = $tck->generic_domain(name => $name1)->uuid($uuid2)->as_xml; # One with a matching UUID, different name my $xml_diffname = $tck->generic_domain(name => $name2)->uuid($uuid1)->as_xml; # One with a different UUID, different name -my $xml_diffboth = $tck->generic_domain(name => $name2)->uuid($uuid2)->as_xml; +my $xml_diffboth = $tck->generic_domain(name => $name2, shareddisk => 1)->uuid($uuid2)->as_xml; diag "Defining persistent domain config"; my ($dom, $dom1); -- 2.17.0