Re: [PATCH] tests/acceptance: test hot(un)plug of ccw devices

[Cornelia Huck]

On Thu, 3 Dec 2020 18:22:35 +0100
Thomas Huth <thuth@redhat.com> wrote:

> On 03/12/2020 16.39, Cornelia Huck wrote:
> > Hotplug a virtio-net-ccw device, and then hotunplug it again.  
> 
> Good idea! ... is it also possible with a pci device?

It should be, I guess; it's not part of my normal test workflow,
however.

> 
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> > 
> > This is on top of "tests/acceptance: enhance s390x devices test"
> > 
> > ---
> >  tests/acceptance/machine_s390_ccw_virtio.py | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/tests/acceptance/machine_s390_ccw_virtio.py 
> > b/tests/acceptance/machine_s390_ccw_virtio.py
> > index 53b8484f8f9c..487c25c31d3c 100644
> > --- a/tests/acceptance/machine_s390_ccw_virtio.py
> > +++ b/tests/acceptance/machine_s390_ccw_virtio.py
> > @@ -97,3 +97,17 @@ class S390CCWVirtioMachine(Test):
> >          exec_command_and_wait_for_pattern(self,
> >                                            'cat 
> > /sys/bus/pci/devices/000a\:00\:00.0/function_id',
> >                                            '0x0000000c')
> > +        # add another device
> > +        self.vm.command('device_add', driver='virtio-net-ccw',
> > +                        devno='fe.0.4711', id='xxx')  
> 
> Could we use a different id, please? xxx sounds so ... well, use your
> imagination.

It is taken straight from my usual testing workflow :) But yeah, I can
call this net_4711 or so.

> 
> > +        exec_command_and_wait_for_pattern(self, 'dmesg | tail -n 1', 
> > 'CRW')  
> 
> That looks like it could be a little bit racy ... what if the kernel outputs
> another log message by chance, so that tail -n 1 reports that instead.
> 
> I think it would be better to clear the dmesg log ("dmesg -c") before
> plugging, and then look at all the new output of "dmesg" without using
> "tail" afterwards.

Yes, good idea. Would need to do the same dance below as well (we get a
new crw).

> 
> > +        exec_command_and_wait_for_pattern(self, 'ls /sys/bus/ccw/devices/',
> > +                                          '0.0.4711')
> > +        # and detach it again
> > +        self.vm.command('device_del', id='xxx')
> > +        self.vm.event_wait(name='DEVICE_DELETED',
> > +                           match={'data': {'device': 'xxx'}})
> > +        exec_command_and_wait_for_pattern(self, 'dmesg | tail -n 1', 
> > 'CRW')  
> 
> dito
> 
> > +        exec_command_and_wait_for_pattern(self,
> > +                                          'ls 
> > /sys/bus/ccw/devices/0.0.4711',
> > +                                          'No such file or directory')
> >   
> 
>  Thomas
> 
> 
> PS: Another idea for a test: Looks like that initrd also has a
> virtio-balloon driver ... we could maybe start with "-device
> virito-balloon", then change the size of the balloon and check whether the
> MemTotal in /proc/meminfo changed...

Added to my to-test list.

Thanks!