On 9/23/19 1:27 PM, Erik Skultety wrote: > The nwfilter 220-no-ip-spoofing.t test relies on an SSH connection to > the test VM. However, because the domain definition passed to libvirt > lacks an RNG device, the SSH server isn't started inside the guest > (even though that is the default on virt-builder images) and therefore: > > "ssh: connect to host 192.168.122.227 port 22: Connection refused" Strange that this has never happened to me. Is it perhaps because I'm using a very old cached image from virt-builder, and had started it up manually at some time in the past (thus giving it a long enough time to generate the keys, which are now stored away for posterity)? > > is returned which in turn makes a bunch of sub tests fail because the > very basic premise - a working SSH connection - is false. > This patch makes sure a virtio RNG is contained in the XML definition, > thus causing the SSH server to be started successfully, thus allowing > all the subtests to pass. > --- > lib/Sys/Virt/TCK.pm | 4 ++++ > lib/Sys/Virt/TCK/DomainBuilder.pm | 21 +++++++++++++++++++++ > 2 files changed, 25 insertions(+) > > diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm > index 389d5cc..3ea06cc 100644 > --- a/lib/Sys/Virt/TCK.pm > +++ b/lib/Sys/Virt/TCK.pm > @@ -807,6 +807,8 @@ sub generic_machine_domain { > $b->disk(src => $config{root}, > dst => $config{dev}, > type => "file"); > + $b->rng(backend_model => "random", > + backend => "/dev/urandom"); > if ($config{firstboot}) { > print "# Running the first boot\n"; > @@ -865,6 +867,8 @@ sub generic_machine_domain { > dst => $config{dev}, > type => "file", > shareable => $shareddisk); > + $b->rng(backend_model => "random", > + backend => "/dev/urandom"); > return $b; > } > } > diff --git a/lib/Sys/Virt/TCK/DomainBuilder.pm b/lib/Sys/Virt/TCK/DomainBuilder.pm > index 45336b5..be8708f 100644 > --- a/lib/Sys/Virt/TCK/DomainBuilder.pm > +++ b/lib/Sys/Virt/TCK/DomainBuilder.pm > @@ -49,6 +49,7 @@ sub new { > graphics => [], > hostdevs => [], > seclabel => {}, > + rng => {}, > }; > bless $self, $class; > @@ -328,6 +329,19 @@ sub seclabel { > return $self; > } > +sub rng { > + my $self = shift; > + my %params = @_; > + > + die "backend model parameter is required" unless $params{backend_model}; > + die "backend parameter is required" unless $params{backend}; > + > + $self->{rng} = \%params; > + $self->{rng}->{model} = "virtio" unless defined $self->{rng}->{model}; > + > + return $self; > +} > + > sub as_xml { > my $self = shift; > @@ -504,6 +518,13 @@ sub as_xml { > $w->endTag("graphics"); > } > $w->emptyTag("console", type => "pty"); > + > + $w->startTag("rng", > + model => $self->{rng}->{model}); > + $w->dataElement("backend", $self->{rng}->{backend}, > + model => $self->{rng}->{backend_model}); > + $w->endTag("rng"); > + Because the <rng> element is added unconditionally, t/070-domain-builder.t fails when you run "./prepare-release.sh"

I fixed this locally by making that bit of code conditional on having backen_model set (see the diff at the end). You could also modify the failing test to explicitly add an <rng> element, but I think we still should put it in conditionally, in case someone needs to *not* have it (e.g., there are xen tests run with libvirt-tck, and they might not like it if the domain had an <rng> element; I can't say that for sure, because I'm not setup to test Xen, and am too lazy to even look at the code and try to figure it out by hand :-)). From my POV, if you apply the diff at the end of this message, then: Reviewed-by: Laine Stump <laine(a)laine.org&gt; You may or may not choose to add an rng to the domain-builder test (and may or may not get a complaint the next time someone runs a Xen test :-) diff --git a/lib/Sys/Virt/TCK/DomainBuilder.pm b/lib/Sys/Virt/TCK/DomainBuilder.pm index be8708f..9e0c49c 100644 --- a/lib/Sys/Virt/TCK/DomainBuilder.pm +++ b/lib/Sys/Virt/TCK/DomainBuilder.pm @@ -519,11 +519,13 @@ sub as_xml {      }      $w->emptyTag("console", type => "pty"); -    $w->startTag("rng", -                 model => $self->{rng}->{model}); -    $w->dataElement("backend", $self->{rng}->{backend}, -                    model => $self->{rng}->{backend_model}); -    $w->endTag("rng"); +    if ($self->{rng}->{backend_model}) {

+        $w->startTag("rng", +                     model => $self->{rng}->{model}); +        $w->dataElement("backend", $self->{rng}->{backend}, +                        model => $self->{rng}->{backend_model}); +        $w->endTag("rng"); +    }      $w->endTag("devices");      if ($self->{seclabel}->{model}) { -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 24, 2019, 2:25 AM Erik Skultety <eskultet(a)redhat.com&gt; wrote: > On Mon, Sep 23, 2019 at 04:47:06PM -0400, Laine Stump wrote: > > On 9/23/19 1:27 PM, Erik Skultety wrote: > > > > From my POV, if you apply the diff at the end of this message, then: > > > > > > Reviewed-by: Laine Stump <laine(a)laine.org&gt; > > > > > > You may or may not choose to add an rng to the domain-builder test (and > may > > or may not get a complaint the next time someone runs a Xen test :-) > > > > > > diff --git a/lib/Sys/Virt/TCK/DomainBuilder.pm > > b/lib/Sys/Virt/TCK/DomainBuilder.pm > > index be8708f..9e0c49c 100644 > > --- a/lib/Sys/Virt/TCK/DomainBuilder.pm > > +++ b/lib/Sys/Virt/TCK/DomainBuilder.pm > > @@ -519,11 +519,13 @@ sub as_xml { > > } > > $w->emptyTag("console", type => "pty"); > > > > - $w->startTag("rng", > > - model => $self->{rng}->{model}); > > - $w->dataElement("backend", $self->{rng}->{backend}, > > - model => $self->{rng}->{backend_model}); > > - $w->endTag("rng"); > > + if ($self->{rng}->{backend_model}) { > > Hmm, wouldn't it be actually better to test for an empty hash instead? IOW > if (%{$self->{rng}}) { > ... > > sounds a bit more generic to me rather than test presence of a specific > attribute within the hash and it seems to be working in context of both the > nwfilter test and the domain builder test with an Xen XML in > prepare_release.sh > If you're okay with that kind of adjustment instead, I'll proceed with > merging > the patch. > Sure, that's fine with me (assuming that you've run prepare-release and the test passes :-)). I was just blindly copying what was done in other code around it.