Re: [PATCH v2 3/4] tests/functional: Test with scripts/vmstate-static-checker.py

[Thomas Huth <thuth@redhat.com>]

On 22/05/2025 16.38, Peter Xu wrote:
> On Thu, May 22, 2025 at 03:37:55PM +0200, Thomas Huth wrote:
> 
> [...]
> 
>> +    def test_vmstate(self):
>> +        target_machine = {
>> +            'aarch64': 'virt-7.2',
>> +            'm68k': 'virt-7.2',
>> +            'ppc64': 'pseries-7.2',
>> +            's390x': 's390-ccw-virtio-7.2',
>> +            'x86_64': 'pc-q35-7.2',
>> +        }
>> +        self.set_machine(target_machine[self.arch])
>> +
>> +        # Run QEMU to get the current vmstate json file:
>> +        dst_json = self.scratch_file('dest.json')
>> +        self.log.info('Dumping vmstate from ' + self.qemu_bin)
>> +        cp = subprocess.run([self.qemu_bin, '-nodefaults',
>> +                             '-M', target_machine[self.arch],
>> +                             '-dump-vmstate', dst_json],
>> +                            stdout=subprocess.PIPE,
>> +                            stderr=subprocess.STDOUT,
>> +                            text=True)
>> +        if cp.returncode != 0:
>> +            self.fail('Running QEMU failed:\n' + cp.stdout)
>> +        if cp.stdout:
>> +            self.log.info('QEMU output: ' + cp.stdout)
>> +
>> +        # Check whether the old vmstate json file is still compatible:
>> +        src_json = self.data_file('..', 'data', 'vmstate-static-checker',
>> +                                  self.arch,
>> +                                  target_machine[self.arch] + '.json')
>> +        self.log.info('Comparing vmstate with ' + src_json)
>> +        cp = self.run_vmstate_checker(src_json, dst_json)
>> +        if cp.returncode != 0:
>> +            self.fail('Running vmstate-static-checker failed:\n' + cp.stdout)
> 
> Would false positives happen here?  Would it fail "make check" and CI, even
> if the change was intended?

Yes. In that case, the quick fix is to remove the problematic piece from the 
7.2 json files. Or we could try to improve the vmstate-static-checker 
script. At least we now notice it immediately, not only after a long delay 
until someone runs the script manually again.

But yes, this can be confusing for the who runs into this problem for the 
first time. I guess I should at least add some friendly words here with 
instructions what has to be done?

  Thomas


>> +        if cp.stdout:
>> +            self.log.warning('vmstate-static-checker output: ' + cp.stdout)
>> +
>> +
>> +if __name__ == '__main__':
>> +    QemuSystemTest.main()
>> -- 
>> 2.49.0
>>
>