Re: [libvirt] [libvirt-tck PATCH] TCK.pm: Define libvirt VMs with an RNG device

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.