/dev/hwrng is only functional if virtio-rng is working right, so let's
add a sanity check for this device node.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/acceptance/machine_s390_ccw_virtio.py | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/tests/acceptance/machine_s390_ccw_virtio.py b/tests/acceptance/machine_s390_ccw_virtio.py
index 733a7ca24a..7d0a78139b 100644
--- a/tests/acceptance/machine_s390_ccw_virtio.py
+++ b/tests/acceptance/machine_s390_ccw_virtio.py
@@ -64,9 +64,9 @@ class S390CCWVirtioMachine(Test):
                          '-append', kernel_command_line,
                          '-device', 'virtio-net-ccw,devno=fe.1.1111',
                          '-device',
-                         'virtio-rng-ccw,devno=fe.2.0000,max_revision=0',
+                         'virtio-rng-ccw,devno=fe.2.0000,max_revision=0,id=rn1',
                          '-device',
-                         'virtio-rng-ccw,devno=fe.3.1234,max_revision=2',
+                         'virtio-rng-ccw,devno=fe.3.1234,max_revision=2,id=rn2',
                          '-device', 'zpci,uid=5,target=zzz',
                          '-device', 'virtio-net-pci,id=zzz',
                          '-device', 'zpci,uid=0xa,fid=12,target=serial',
@@ -96,6 +96,19 @@ class S390CCWVirtioMachine(Test):
         exec_command_and_wait_for_pattern(self,
                         'cat /sys/bus/ccw/devices/0.3.1234/virtio?/features',
                         virtio_rng_features)
+        # check that /dev/hwrng works - and that it's gone after ejecting
+        exec_command_and_wait_for_pattern(self,
+                        'dd if=/dev/hwrng of=/tmp/out.dat bs=1k count=10',
+                        '10+0 records out')
+        self.clear_guests_dmesg()
+        self.vm.command('device_del', id='rn1')
+        self.wait_for_crw_reports()
+        self.clear_guests_dmesg()
+        self.vm.command('device_del', id='rn2')
+        self.wait_for_crw_reports()
+        exec_command_and_wait_for_pattern(self,
+                        'dd if=/dev/hwrng of=/tmp/out.dat bs=1k count=10',
+                        'dd: /dev/hwrng: No such device')
         # verify that we indeed have virtio-net devices (without having the
         # virtio-net driver handy)
         exec_command_and_wait_for_pattern(self,
-- 
2.27.0

Hi,

On 12/11/20 2:31 PM, Thomas Huth wrote:
> /dev/hwrng is only functional if virtio-rng is working right, so let's
> add a sanity check for this device node.

Good idea.

>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/acceptance/machine_s390_ccw_virtio.py | 17 +++++++++++++++--
>   1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/tests/acceptance/machine_s390_ccw_virtio.py b/tests/acceptance/machine_s390_ccw_virtio.py
> index 733a7ca24a..7d0a78139b 100644
> --- a/tests/acceptance/machine_s390_ccw_virtio.py
> +++ b/tests/acceptance/machine_s390_ccw_virtio.py
> @@ -64,9 +64,9 @@ class S390CCWVirtioMachine(Test):
>                            '-append', kernel_command_line,
>                            '-device', 'virtio-net-ccw,devno=fe.1.1111',
>                            '-device',
> -                         'virtio-rng-ccw,devno=fe.2.0000,max_revision=0',
> +                         'virtio-rng-ccw,devno=fe.2.0000,max_revision=0,id=rn1',
>                            '-device',
> -                         'virtio-rng-ccw,devno=fe.3.1234,max_revision=2',
> +                         'virtio-rng-ccw,devno=fe.3.1234,max_revision=2,id=rn2',
>                            '-device', 'zpci,uid=5,target=zzz',
>                            '-device', 'virtio-net-pci,id=zzz',
>                            '-device', 'zpci,uid=0xa,fid=12,target=serial',
> @@ -96,6 +96,19 @@ class S390CCWVirtioMachine(Test):
>           exec_command_and_wait_for_pattern(self,
>                           'cat /sys/bus/ccw/devices/0.3.1234/virtio?/features',
>                           virtio_rng_features)
> +        # check that /dev/hwrng works - and that it's gone after ejecting
> +        exec_command_and_wait_for_pattern(self,
> +                        'dd if=/dev/hwrng of=/tmp/out.dat bs=1k count=10',
> +                        '10+0 records out')
> +        self.clear_guests_dmesg()
> +        self.vm.command('device_del', id='rn1')
> +        self.wait_for_crw_reports()
> +        self.clear_guests_dmesg()
> +        self.vm.command('device_del', id='rn2')
> +        self.wait_for_crw_reports()
> +        exec_command_and_wait_for_pattern(self,
> +                        'dd if=/dev/hwrng of=/tmp/out.dat bs=1k count=10',
> +                        'dd: /dev/hwrng: No such device')

Maybe the expected pattern is too fragile. On my Fedora 33 system, 'dd' 
will print a different message.

What if it checks for the presence of the device file, e.g:

... self, 'test -c /dev/hwrng; echo $?', '1')

- Wainer

>           # verify that we indeed have virtio-net devices (without having the
>           # virtio-net driver handy)
>           exec_command_and_wait_for_pattern(self,

On 11/12/2020 21.30, Wainer dos Santos Moschetta wrote:
> Hi,
> 
> On 12/11/20 2:31 PM, Thomas Huth wrote:
>> /dev/hwrng is only functional if virtio-rng is working right, so let's
>> add a sanity check for this device node.
> 
> Good idea.
> 
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   tests/acceptance/machine_s390_ccw_virtio.py | 17 +++++++++++++++--
>>   1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/acceptance/machine_s390_ccw_virtio.py
>> b/tests/acceptance/machine_s390_ccw_virtio.py
>> index 733a7ca24a..7d0a78139b 100644
>> --- a/tests/acceptance/machine_s390_ccw_virtio.py
>> +++ b/tests/acceptance/machine_s390_ccw_virtio.py
>> @@ -64,9 +64,9 @@ class S390CCWVirtioMachine(Test):
>>                            '-append', kernel_command_line,
>>                            '-device', 'virtio-net-ccw,devno=fe.1.1111',
>>                            '-device',
>> -                         'virtio-rng-ccw,devno=fe.2.0000,max_revision=0',
>> +                        
>> 'virtio-rng-ccw,devno=fe.2.0000,max_revision=0,id=rn1',
>>                            '-device',
>> -                         'virtio-rng-ccw,devno=fe.3.1234,max_revision=2',
>> +                        
>> 'virtio-rng-ccw,devno=fe.3.1234,max_revision=2,id=rn2',
>>                            '-device', 'zpci,uid=5,target=zzz',
>>                            '-device', 'virtio-net-pci,id=zzz',
>>                            '-device', 'zpci,uid=0xa,fid=12,target=serial',
>> @@ -96,6 +96,19 @@ class S390CCWVirtioMachine(Test):
>>           exec_command_and_wait_for_pattern(self,
>>                           'cat
>> /sys/bus/ccw/devices/0.3.1234/virtio?/features',
>>                           virtio_rng_features)
>> +        # check that /dev/hwrng works - and that it's gone after ejecting
>> +        exec_command_and_wait_for_pattern(self,
>> +                        'dd if=/dev/hwrng of=/tmp/out.dat bs=1k count=10',
>> +                        '10+0 records out')
>> +        self.clear_guests_dmesg()
>> +        self.vm.command('device_del', id='rn1')
>> +        self.wait_for_crw_reports()
>> +        self.clear_guests_dmesg()
>> +        self.vm.command('device_del', id='rn2')
>> +        self.wait_for_crw_reports()
>> +        exec_command_and_wait_for_pattern(self,
>> +                        'dd if=/dev/hwrng of=/tmp/out.dat bs=1k count=10',
>> +                        'dd: /dev/hwrng: No such device')
> 
> Maybe the expected pattern is too fragile. On my Fedora 33 system, 'dd' will
> print a different message.

We are running this test with a well-defined kernel + initrd, so I don't
think we have to care of other versions of dd here.

> What if it checks for the presence of the device file, e.g:
> 
> ... self, 'test -c /dev/hwrng; echo $?', '1')

That doesn't work, the /dev/hwrng is still there (so test -c succeeds),
since this initrd uses static device nodes for this in /dev. /dev/hwrng just
can not be opened anymore after the device has been removed.

 Thomas

On Sat, 12 Dec 2020 08:10:01 +0100
Thomas Huth <thuth@redhat.com> wrote:

Minor nit: I think that the subject is a bit unwieldy. What about

"tests/acceptance: Test virtio-rng on s390 via /dev/hwrng"

?

> On 11/12/2020 21.30, Wainer dos Santos Moschetta wrote:
> > Hi,
> > 
> > On 12/11/20 2:31 PM, Thomas Huth wrote:  
> >> /dev/hwrng is only functional if virtio-rng is working right, so let's
> >> add a sanity check for this device node.  
> > 
> > Good idea.
> >   
> >>
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >> ---
> >>   tests/acceptance/machine_s390_ccw_virtio.py | 17 +++++++++++++++--
> >>   1 file changed, 15 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tests/acceptance/machine_s390_ccw_virtio.py
> >> b/tests/acceptance/machine_s390_ccw_virtio.py
> >> index 733a7ca24a..7d0a78139b 100644
> >> --- a/tests/acceptance/machine_s390_ccw_virtio.py
> >> +++ b/tests/acceptance/machine_s390_ccw_virtio.py
> >> @@ -64,9 +64,9 @@ class S390CCWVirtioMachine(Test):
> >>                            '-append', kernel_command_line,
> >>                            '-device', 'virtio-net-ccw,devno=fe.1.1111',
> >>                            '-device',
> >> -                         'virtio-rng-ccw,devno=fe.2.0000,max_revision=0',
> >> +                        
> >> 'virtio-rng-ccw,devno=fe.2.0000,max_revision=0,id=rn1',
> >>                            '-device',
> >> -                         'virtio-rng-ccw,devno=fe.3.1234,max_revision=2',
> >> +                        
> >> 'virtio-rng-ccw,devno=fe.3.1234,max_revision=2,id=rn2',
> >>                            '-device', 'zpci,uid=5,target=zzz',
> >>                            '-device', 'virtio-net-pci,id=zzz',
> >>                            '-device', 'zpci,uid=0xa,fid=12,target=serial',
> >> @@ -96,6 +96,19 @@ class S390CCWVirtioMachine(Test):
> >>           exec_command_and_wait_for_pattern(self,
> >>                           'cat
> >> /sys/bus/ccw/devices/0.3.1234/virtio?/features',
> >>                           virtio_rng_features)
> >> +        # check that /dev/hwrng works - and that it's gone after ejecting
> >> +        exec_command_and_wait_for_pattern(self,
> >> +                        'dd if=/dev/hwrng of=/tmp/out.dat bs=1k count=10',
> >> +                        '10+0 records out')
> >> +        self.clear_guests_dmesg()
> >> +        self.vm.command('device_del', id='rn1')
> >> +        self.wait_for_crw_reports()
> >> +        self.clear_guests_dmesg()
> >> +        self.vm.command('device_del', id='rn2')
> >> +        self.wait_for_crw_reports()
> >> +        exec_command_and_wait_for_pattern(self,
> >> +                        'dd if=/dev/hwrng of=/tmp/out.dat bs=1k count=10',

Does this work if you direct the output to /dev/null?

> >> +                        'dd: /dev/hwrng: No such device')  
> > 
> > Maybe the expected pattern is too fragile. On my Fedora 33 system, 'dd' will
> > print a different message.  
> 
> We are running this test with a well-defined kernel + initrd, so I don't
> think we have to care of other versions of dd here.
> 
> > What if it checks for the presence of the device file, e.g:
> > 
> > ... self, 'test -c /dev/hwrng; echo $?', '1')  
> 
> That doesn't work, the /dev/hwrng is still there (so test -c succeeds),
> since this initrd uses static device nodes for this in /dev. /dev/hwrng just
> can not be opened anymore after the device has been removed.

I had been thinking about a different approach to check that, but dd
really looks like the easiest way.

On 12/12/20 4:10 AM, Thomas Huth wrote:
> On 11/12/2020 21.30, Wainer dos Santos Moschetta wrote:
>> Hi,
>>
>> On 12/11/20 2:31 PM, Thomas Huth wrote:
>>> /dev/hwrng is only functional if virtio-rng is working right, so let's
>>> add a sanity check for this device node.
>> Good idea.
>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>    tests/acceptance/machine_s390_ccw_virtio.py | 17 +++++++++++++++--
>>>    1 file changed, 15 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/acceptance/machine_s390_ccw_virtio.py
>>> b/tests/acceptance/machine_s390_ccw_virtio.py
>>> index 733a7ca24a..7d0a78139b 100644
>>> --- a/tests/acceptance/machine_s390_ccw_virtio.py
>>> +++ b/tests/acceptance/machine_s390_ccw_virtio.py
>>> @@ -64,9 +64,9 @@ class S390CCWVirtioMachine(Test):
>>>                             '-append', kernel_command_line,
>>>                             '-device', 'virtio-net-ccw,devno=fe.1.1111',
>>>                             '-device',
>>> -                         'virtio-rng-ccw,devno=fe.2.0000,max_revision=0',
>>> +
>>> 'virtio-rng-ccw,devno=fe.2.0000,max_revision=0,id=rn1',
>>>                             '-device',
>>> -                         'virtio-rng-ccw,devno=fe.3.1234,max_revision=2',
>>> +
>>> 'virtio-rng-ccw,devno=fe.3.1234,max_revision=2,id=rn2',
>>>                             '-device', 'zpci,uid=5,target=zzz',
>>>                             '-device', 'virtio-net-pci,id=zzz',
>>>                             '-device', 'zpci,uid=0xa,fid=12,target=serial',
>>> @@ -96,6 +96,19 @@ class S390CCWVirtioMachine(Test):
>>>            exec_command_and_wait_for_pattern(self,
>>>                            'cat
>>> /sys/bus/ccw/devices/0.3.1234/virtio?/features',
>>>                            virtio_rng_features)
>>> +        # check that /dev/hwrng works - and that it's gone after ejecting
>>> +        exec_command_and_wait_for_pattern(self,
>>> +                        'dd if=/dev/hwrng of=/tmp/out.dat bs=1k count=10',
>>> +                        '10+0 records out')
>>> +        self.clear_guests_dmesg()
>>> +        self.vm.command('device_del', id='rn1')
>>> +        self.wait_for_crw_reports()
>>> +        self.clear_guests_dmesg()
>>> +        self.vm.command('device_del', id='rn2')
>>> +        self.wait_for_crw_reports()
>>> +        exec_command_and_wait_for_pattern(self,
>>> +                        'dd if=/dev/hwrng of=/tmp/out.dat bs=1k count=10',
>>> +                        'dd: /dev/hwrng: No such device')
>> Maybe the expected pattern is too fragile. On my Fedora 33 system, 'dd' will
>> print a different message.
> We are running this test with a well-defined kernel + initrd, so I don't
> think we have to care of other versions of dd here.

I was thinking in the case someone change the kernel+initrd for this 
test. Well, in that case the same person will have to make the 
adjustments. So,

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

>
>> What if it checks for the presence of the device file, e.g:
>>
>> ... self, 'test -c /dev/hwrng; echo $?', '1')
> That doesn't work, the /dev/hwrng is still there (so test -c succeeds),
> since this initrd uses static device nodes for this in /dev. /dev/hwrng just
> can not be opened anymore after the device has been removed.
>
>   Thomas