[PATCH 0/3] tests/acceptance: Test virtio-rng and -balloon on s390x

Thomas Huth posted 3 patches 3 years, 4 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
tests/acceptance/machine_s390_ccw_virtio.py | 59 +++++++++++++++------
1 file changed, 43 insertions(+), 16 deletions(-)
[PATCH 0/3] tests/acceptance: Test virtio-rng and -balloon on s390x
Posted by Thomas Huth 3 years, 4 months ago
Add two more simple tests to check that virtio-rng and virtio-balloon
are at least (very) basically working on s390x.

Based-on: 20201204121450.120730-1-cohuck@redhat.com

Thomas Huth (3):
  tests/acceptance: Extract the code to clear dmesg and wait for CRW
    reports
  tests/acceptance/machine_s390_ccw_virtio: Test virtio-rng via
    /dev/hwrng
  tests/acceptance/machine_s390_ccw_virtio: Test the virtio-balloon
    device

 tests/acceptance/machine_s390_ccw_virtio.py | 59 +++++++++++++++------
 1 file changed, 43 insertions(+), 16 deletions(-)

-- 
2.27.0


Re: [PATCH 0/3] tests/acceptance: Test virtio-rng and -balloon on s390x
Posted by Willian Rampazzo 3 years, 4 months ago
On 12/11/20 2:31 PM, Thomas Huth wrote:
> Add two more simple tests to check that virtio-rng and virtio-balloon
> are at least (very) basically working on s390x.
> 
> Based-on: 20201204121450.120730-1-cohuck@redhat.com
> 
> Thomas Huth (3):
>    tests/acceptance: Extract the code to clear dmesg and wait for CRW
>      reports
>    tests/acceptance/machine_s390_ccw_virtio: Test virtio-rng via
>      /dev/hwrng
>    tests/acceptance/machine_s390_ccw_virtio: Test the virtio-balloon
>      device
> 
>   tests/acceptance/machine_s390_ccw_virtio.py | 59 +++++++++++++++------
>   1 file changed, 43 insertions(+), 16 deletions(-)
> 

One observation, test_s390x_devices tends to get longer and difficult to 
debug in case of problems. If a test covers one specific device type, It 
will improve readability, flexibility, and debugging. In case you don't 
want to spend time breaking this into multiple tests, I'll be glad to do 
that after the whole series is merged.

As far as code concerned,

Reviewed-by: Willian Rampazzo <willianr@redhat.com>
Tested-by: Willian Rampazzo <willianr@redhat.com>

Fetching asset from 
tests/acceptance/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_devices
Fetching asset from 
tests/acceptance/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_devices
JOB ID     : 8ba8e572f2582f9a48f2542423342e51e257db97
JOB LOG    : 
/home/linux1/src/qemu.dev/build/tests/results/job-2020-12-11T15.01-8ba8e57/job.log
  (1/1) 
tests/acceptance/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_devices: 
PASS (7.89 s)
RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
CANCEL 0
JOB TIME   : 8.03 s


Re: [PATCH 0/3] tests/acceptance: Test virtio-rng and -balloon on s390x
Posted by Thomas Huth 3 years, 4 months ago
On 11/12/2020 21.10, Willian Rampazzo wrote:
> On 12/11/20 2:31 PM, Thomas Huth wrote:
>> Add two more simple tests to check that virtio-rng and virtio-balloon
>> are at least (very) basically working on s390x.
>>
>> Based-on: 20201204121450.120730-1-cohuck@redhat.com
>>
>> Thomas Huth (3):
>>    tests/acceptance: Extract the code to clear dmesg and wait for CRW
>>      reports
>>    tests/acceptance/machine_s390_ccw_virtio: Test virtio-rng via
>>      /dev/hwrng
>>    tests/acceptance/machine_s390_ccw_virtio: Test the virtio-balloon
>>      device
>>
>>   tests/acceptance/machine_s390_ccw_virtio.py | 59 +++++++++++++++------
>>   1 file changed, 43 insertions(+), 16 deletions(-)
>>
> 
> One observation, test_s390x_devices tends to get longer and difficult to
> debug in case of problems. If a test covers one specific device type, It
> will improve readability, flexibility, and debugging. In case you don't want
> to spend time breaking this into multiple tests, I'll be glad to do that
> after the whole series is merged.

Theoretically yes, but practically we also want to run the tests as fast as
possible. Quite a bit of time is used to boot the kernel, so if we add a new
test for each and every device, that would increase the test time quite a
bit. Thus I'd rather prefer to keep everything in one single test instead
for now.

> As far as code concerned,
> 
> Reviewed-by: Willian Rampazzo <willianr@redhat.com>
> Tested-by: Willian Rampazzo <willianr@redhat.com>

Thanks!

 Thomas


Re: [PATCH 0/3] tests/acceptance: Test virtio-rng and -balloon on s390x
Posted by Cornelia Huck 3 years, 4 months ago
On Sat, 12 Dec 2020 08:15:13 +0100
Thomas Huth <thuth@redhat.com> wrote:

> On 11/12/2020 21.10, Willian Rampazzo wrote:
> > On 12/11/20 2:31 PM, Thomas Huth wrote:  
> >> Add two more simple tests to check that virtio-rng and virtio-balloon
> >> are at least (very) basically working on s390x.
> >>
> >> Based-on: 20201204121450.120730-1-cohuck@redhat.com
> >>
> >> Thomas Huth (3):
> >>    tests/acceptance: Extract the code to clear dmesg and wait for CRW
> >>      reports
> >>    tests/acceptance/machine_s390_ccw_virtio: Test virtio-rng via
> >>      /dev/hwrng
> >>    tests/acceptance/machine_s390_ccw_virtio: Test the virtio-balloon
> >>      device
> >>
> >>   tests/acceptance/machine_s390_ccw_virtio.py | 59 +++++++++++++++------
> >>   1 file changed, 43 insertions(+), 16 deletions(-)
> >>  
> > 
> > One observation, test_s390x_devices tends to get longer and difficult to
> > debug in case of problems. If a test covers one specific device type, It
> > will improve readability, flexibility, and debugging. In case you don't want
> > to spend time breaking this into multiple tests, I'll be glad to do that
> > after the whole series is merged.  
> 
> Theoretically yes, but practically we also want to run the tests as fast as
> possible. Quite a bit of time is used to boot the kernel, so if we add a new
> test for each and every device, that would increase the test time quite a
> bit. Thus I'd rather prefer to keep everything in one single test instead
> for now.

I agree.

Is there any other way to make this more debuggable?


Re: [PATCH 0/3] tests/acceptance: Test virtio-rng and -balloon on s390x
Posted by Willian Rampazzo 3 years, 4 months ago
On Mon, Dec 14, 2020 at 8:39 AM Cornelia Huck <cohuck@redhat.com> wrote:
>
> On Sat, 12 Dec 2020 08:15:13 +0100
> Thomas Huth <thuth@redhat.com> wrote:
>
> > On 11/12/2020 21.10, Willian Rampazzo wrote:
> > > On 12/11/20 2:31 PM, Thomas Huth wrote:
> > >> Add two more simple tests to check that virtio-rng and virtio-balloon
> > >> are at least (very) basically working on s390x.
> > >>
> > >> Based-on: 20201204121450.120730-1-cohuck@redhat.com
> > >>
> > >> Thomas Huth (3):
> > >>    tests/acceptance: Extract the code to clear dmesg and wait for CRW
> > >>      reports
> > >>    tests/acceptance/machine_s390_ccw_virtio: Test virtio-rng via
> > >>      /dev/hwrng
> > >>    tests/acceptance/machine_s390_ccw_virtio: Test the virtio-balloon
> > >>      device
> > >>
> > >>   tests/acceptance/machine_s390_ccw_virtio.py | 59 +++++++++++++++------
> > >>   1 file changed, 43 insertions(+), 16 deletions(-)
> > >>
> > >
> > > One observation, test_s390x_devices tends to get longer and difficult to
> > > debug in case of problems. If a test covers one specific device type, It
> > > will improve readability, flexibility, and debugging. In case you don't want
> > > to spend time breaking this into multiple tests, I'll be glad to do that
> > > after the whole series is merged.
> >
> > Theoretically yes, but practically we also want to run the tests as fast as
> > possible. Quite a bit of time is used to boot the kernel, so if we add a new
> > test for each and every device, that would increase the test time quite a
> > bit. Thus I'd rather prefer to keep everything in one single test instead
> > for now.
>
> I agree.
>
> Is there any other way to make this more debuggable?

I have some ideas in mind but I need to run some tests. I'm fine
merging the current code and improving it later.


Re: [PATCH 0/3] tests/acceptance: Test virtio-rng and -balloon on s390x
Posted by Willian Rampazzo 3 years, 4 months ago
On Sat, Dec 12, 2020 at 4:15 AM Thomas Huth <thuth@redhat.com> wrote:
>
> On 11/12/2020 21.10, Willian Rampazzo wrote:
> > On 12/11/20 2:31 PM, Thomas Huth wrote:
> >> Add two more simple tests to check that virtio-rng and virtio-balloon
> >> are at least (very) basically working on s390x.
> >>
> >> Based-on: 20201204121450.120730-1-cohuck@redhat.com
> >>
> >> Thomas Huth (3):
> >>    tests/acceptance: Extract the code to clear dmesg and wait for CRW
> >>      reports
> >>    tests/acceptance/machine_s390_ccw_virtio: Test virtio-rng via
> >>      /dev/hwrng
> >>    tests/acceptance/machine_s390_ccw_virtio: Test the virtio-balloon
> >>      device
> >>
> >>   tests/acceptance/machine_s390_ccw_virtio.py | 59 +++++++++++++++------
> >>   1 file changed, 43 insertions(+), 16 deletions(-)
> >>
> >
> > One observation, test_s390x_devices tends to get longer and difficult to
> > debug in case of problems. If a test covers one specific device type, It
> > will improve readability, flexibility, and debugging. In case you don't want
> > to spend time breaking this into multiple tests, I'll be glad to do that
> > after the whole series is merged.
>
> Theoretically yes, but practically we also want to run the tests as fast as
> possible. Quite a bit of time is used to boot the kernel, so if we add a new
> test for each and every device, that would increase the test time quite a
> bit. Thus I'd rather prefer to keep everything in one single test instead
> for now.

That is, indeed, a good point!


>
> > As far as code concerned,
> >
> > Reviewed-by: Willian Rampazzo <willianr@redhat.com>
> > Tested-by: Willian Rampazzo <willianr@redhat.com>
>
> Thanks!
>
>  Thomas
>