On Mon, 7 Dec 2020 15:28:47 +0100
Thomas Huth <th...@redhat.com> wrote:
> On 04/12/2020 13.14, Cornelia Huck wrote:
> > Hotplug a virtio-net-ccw device, and then hotunplug it again.
> >
> > Signed-off-by: Cornelia Huck <coh...@redhat.com>
> > ---
> >
> > v1->v2:
> > - switch device id
> > - clear out dmesg before looking for CRW messages
> >
> > ---
> > tests/acceptance/machine_s390_ccw_virtio.py | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/tests/acceptance/machine_s390_ccw_virtio.py
> > b/tests/acceptance/machine_s390_ccw_virtio.py
> > index 53b8484f8f9c..83c00190621b 100644
> > --- a/tests/acceptance/machine_s390_ccw_virtio.py
> > +++ b/tests/acceptance/machine_s390_ccw_virtio.py
> > @@ -97,3 +97,19 @@ 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
> > + exec_command_and_wait_for_pattern(self, 'dmesg -c', ' ')
> > + self.vm.command('device_add', driver='virtio-net-ccw',
> > + devno='fe.0.4711', id='net_4711')
> > + exec_command_and_wait_for_pattern(self, 'dmesg', 'CRW')
>
> Looking at this twice, I'm a little bit afraid that this could be racy -
> what if the kernel decides to emit the line with the "CRW" just after we
> executed the dmesg command? I'd maybe use something like this instead:
>
> exec_command_and_wait_for_pattern(self,
> 'while ! dmesg -c | grep CRW ; do sleep 1 ; done', '~ #')
Yes, you're right. Unless anyone can think of a better incantation?
>
> > + exec_command_and_wait_for_pattern(self, 'ls /sys/bus/ccw/devices/',
> > + '0.0.4711')
> > + # and detach it again
> > + exec_command_and_wait_for_pattern(self, 'dmesg -c', ' ')
>
> If adapt my above change, you could also get rid of this dmesg -c here
> (since it's done in the while loop already)
I don't think so (there are two CRWs posted, and the loop might have
caught the first one only.)
>
> > + self.vm.command('device_del', id='net_4711')
> > + self.vm.event_wait(name='DEVICE_DELETED',
> > + match={'data': {'device': 'net_4711'}})
> > + exec_command_and_wait_for_pattern(self, 'dmesg', 'CRW')
>
> dito
>
> > + exec_command_and_wait_for_pattern(self,
> > + 'ls
> > /sys/bus/ccw/devices/0.0.4711',
> > + 'No such file or directory')
>
> With my suggestion applied:
>
> Reviewed-by: Thomas Huth <th...@redhat.com>
