Re: [PATCH v3 8/8] tests/functional: Add test for fadump in PSeries

[Thomas Huth <thuth@redhat.com>]

On 17/03/2025 07.51, Aditya Gupta wrote:
> Hi Thomas,
> 
> On 17/03/25 11:55, Thomas Huth wrote:
>>  Hi!
>>
>> On 15/03/2025 07.46, Aditya Gupta wrote:
>>> Add testcases for testing fadump with PSeries and PSeries+KVM
>>> combinations
>> ...
>>> diff --git a/tests/functional/test_ppc64_fadump.py b/tests/functional/ 
>>> test_ppc64_fadump.py
>>> new file mode 100755
>>> index 000000000000..3d6d3734e243
>>> --- /dev/null
>>> +++ b/tests/functional/test_ppc64_fadump.py
>>> @@ -0,0 +1,185 @@
>>> +#!/usr/bin/env python3
>>
>> scripts/checkpatch.pl recently got a check for SPDX license tags, so 
>> please add a SPDX-License-Identifier here now to avoid a warning when the 
>> script is run with your patch.
>>
> Sure, don't know how I missed it. Will fix it.
> 
> 
>>> <...snip...>
>>>
>>> +    @skipUnless(platform.machine().startswith("ppc64"),
>>> +                "KVM tests require the same host and guest architecture")
>>
>> I think this is likely unreliable: The test could run on a ppc64 host, but 
>> KVM could still be unavailable, e.g. if it has been disabled in the 
>> kernel. It's better to use self.require_accelerator("kvm") instead (which 
>> will also skip the test if it is not available).
>>
> Thanks, agreed, will remove it, and add self.require_accelerator("kvm").
>>
>> Also, shouldn't there be a "-accel kvm" somewhere if you really want to be 
>> sure to enable KVM ?
>>
> Will add it, this whole time I thought it's using kvm, guess the tests were 
> running in tcg mode only.
>>> +    def test_fadump_pseries_kvm(self):
>>> +        """
>>> +        Test Fadump in PSeries with KVM accel
>>> +        """
>>> +        self.do_test_fadump(is_kvm=True, is_powernv=False)
>>> +
>>> +if __name__ == '__main__':
>>> +    if os.getenv("DEBUG"):
>>> +        logging.basicConfig(level=logging.DEBUG)
>>
>> The setUp function in QemuSystemTest already sets the log level to DEBUG, 
>> so this should not be necessary?
>>
> I did that to see the QEMU VM booting logs. I see in QemuSystemTest, it sets 
> the log level of some loggers to DEBUG, don't know if it's the same, as I 
> didn't see the boot logs during testing (also, in QemuSystemTest, we are 
> setting specific loggers to DEBUG, while I did .basicConfig to DEBUG which I 
> think is like a global debug level setting)
> 
> How do I get the system booting logs in `make check` ? I didn't find it in 
> the docs.

The log files should get written to 
tests/functional/ppc64/test_ppc64_fadump.* - main log file is base.log, and 
the console output should go into console.log. Can you spot there the 
information you were looking for?

Anyway, that information is missing in the docs indeed ... I'll try to come 
up with a patch when I got some spare time for it.

  Thomas