[PATCH] scripts/vmstate-static-checker: Fix deprecation warnings with latest argparse

Thomas Huth posted 1 patch 2 weeks, 1 day ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251030092638.39505-1-thuth@redhat.com
Maintainers: John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>
scripts/vmstate-static-checker.py | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
[PATCH] scripts/vmstate-static-checker: Fix deprecation warnings with latest argparse
Posted by Thomas Huth 2 weeks, 1 day ago
From: Thomas Huth <thuth@redhat.com>

The argparse.FileType() type has been deprecated in the latest argparse
version (e.g. the one from Fedora 43), now causing the test_bad_vmstate
functional test to fail since there are unexpected strings in the output.
Change the script to use pathlib.Path instead to fix the test_bad_vmstate
test and to be prepared for the future when the deprecated FileType gets
removed completely.

Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 scripts/vmstate-static-checker.py | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/scripts/vmstate-static-checker.py b/scripts/vmstate-static-checker.py
index 2335e25f94c..89b100e6cca 100755
--- a/scripts/vmstate-static-checker.py
+++ b/scripts/vmstate-static-checker.py
@@ -21,6 +21,7 @@
 
 import argparse
 import json
+import pathlib
 import sys
 
 # Count the number of errors found
@@ -382,10 +383,10 @@ def main():
     help_text = "Parse JSON-formatted vmstate dumps from QEMU in files SRC and DEST.  Checks whether migration from SRC to DEST QEMU versions would break based on the VMSTATE information contained within the JSON outputs.  The JSON output is created from a QEMU invocation with the -dump-vmstate parameter and a filename argument to it.  Other parameters to QEMU do not matter, except the -M (machine type) parameter."
 
     parser = argparse.ArgumentParser(description=help_text)
-    parser.add_argument('-s', '--src', type=argparse.FileType('r'),
+    parser.add_argument('-s', '--src', type=pathlib.Path,
                         required=True,
                         help='json dump from src qemu')
-    parser.add_argument('-d', '--dest', type=argparse.FileType('r'),
+    parser.add_argument('-d', '--dest', type=pathlib.Path,
                         required=True,
                         help='json dump from dest qemu')
     parser.add_argument('--reverse', required=False, default=False,
@@ -393,10 +394,10 @@ def main():
                         help='reverse the direction')
     args = parser.parse_args()
 
-    src_data = json.load(args.src)
-    dest_data = json.load(args.dest)
-    args.src.close()
-    args.dest.close()
+    with open(args.src, 'r', encoding='utf-8') as src_fh:
+        src_data = json.load(src_fh)
+    with open(args.dest, 'r', encoding='utf-8') as dst_fh:
+        dest_data = json.load(dst_fh)
 
     if args.reverse:
         temp = src_data
-- 
2.51.0


Re: [PATCH] scripts/vmstate-static-checker: Fix deprecation warnings with latest argparse
Posted by Peter Xu 2 weeks, 1 day ago
On Thu, Oct 30, 2025 at 10:26:38AM +0100, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> The argparse.FileType() type has been deprecated in the latest argparse
> version (e.g. the one from Fedora 43), now causing the test_bad_vmstate
> functional test to fail since there are unexpected strings in the output.
> Change the script to use pathlib.Path instead to fix the test_bad_vmstate
> test and to be prepared for the future when the deprecated FileType gets
> removed completely.
> 
> Reported-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

queued, thanks.

-- 
Peter Xu


Re: [PATCH] scripts/vmstate-static-checker: Fix deprecation warnings with latest argparse
Posted by Daniel P. Berrangé 2 weeks, 1 day ago
On Thu, Oct 30, 2025 at 10:26:38AM +0100, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> The argparse.FileType() type has been deprecated in the latest argparse
> version (e.g. the one from Fedora 43), now causing the test_bad_vmstate
> functional test to fail since there are unexpected strings in the output.
> Change the script to use pathlib.Path instead to fix the test_bad_vmstate
> test and to be prepared for the future when the deprecated FileType gets
> removed completely.
> 
> Reported-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  scripts/vmstate-static-checker.py | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)

AFAICT, using pathlib.Path should work going back to any old python
versions we would need.

> 
> diff --git a/scripts/vmstate-static-checker.py b/scripts/vmstate-static-checker.py
> index 2335e25f94c..89b100e6cca 100755
> --- a/scripts/vmstate-static-checker.py
> +++ b/scripts/vmstate-static-checker.py
> @@ -21,6 +21,7 @@
>  
>  import argparse
>  import json
> +import pathlib
>  import sys
>  
>  # Count the number of errors found
> @@ -382,10 +383,10 @@ def main():
>      help_text = "Parse JSON-formatted vmstate dumps from QEMU in files SRC and DEST.  Checks whether migration from SRC to DEST QEMU versions would break based on the VMSTATE information contained within the JSON outputs.  The JSON output is created from a QEMU invocation with the -dump-vmstate parameter and a filename argument to it.  Other parameters to QEMU do not matter, except the -M (machine type) parameter."
>  
>      parser = argparse.ArgumentParser(description=help_text)
> -    parser.add_argument('-s', '--src', type=argparse.FileType('r'),
> +    parser.add_argument('-s', '--src', type=pathlib.Path,
>                          required=True,
>                          help='json dump from src qemu')
> -    parser.add_argument('-d', '--dest', type=argparse.FileType('r'),
> +    parser.add_argument('-d', '--dest', type=pathlib.Path,
>                          required=True,
>                          help='json dump from dest qemu')
>      parser.add_argument('--reverse', required=False, default=False,
> @@ -393,10 +394,10 @@ def main():
>                          help='reverse the direction')
>      args = parser.parse_args()
>  
> -    src_data = json.load(args.src)
> -    dest_data = json.load(args.dest)
> -    args.src.close()
> -    args.dest.close()
> +    with open(args.src, 'r', encoding='utf-8') as src_fh:
> +        src_data = json.load(src_fh)
> +    with open(args.dest, 'r', encoding='utf-8') as dst_fh:
> +        dest_data = json.load(dst_fh)

This could be

 src_data = json.load(args.src.read_text('utf-8'))
 dest_data = json.load(args.dest.read_text('utf-8'))


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH] scripts/vmstate-static-checker: Fix deprecation warnings with latest argparse
Posted by Thomas Huth 2 weeks, 1 day ago
On 30/10/2025 10.33, Daniel P. Berrangé wrote:
> On Thu, Oct 30, 2025 at 10:26:38AM +0100, Thomas Huth wrote:
>> From: Thomas Huth <thuth@redhat.com>
>>
>> The argparse.FileType() type has been deprecated in the latest argparse
>> version (e.g. the one from Fedora 43), now causing the test_bad_vmstate
>> functional test to fail since there are unexpected strings in the output.
>> Change the script to use pathlib.Path instead to fix the test_bad_vmstate
>> test and to be prepared for the future when the deprecated FileType gets
>> removed completely.
>>
>> Reported-by: Daniel P. Berrangé <berrange@redhat.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   scripts/vmstate-static-checker.py | 13 +++++++------
>>   1 file changed, 7 insertions(+), 6 deletions(-)
> 
> AFAICT, using pathlib.Path should work going back to any old python
> versions we would need.

According to https://docs.python.org/3/library/pathlib.html it has been 
added with Python 3.4, and we require at least 3.9 nowadays, so yes, this 
should be fine.

>> @@ -393,10 +394,10 @@ def main():
>>                           help='reverse the direction')
>>       args = parser.parse_args()
>>   
>> -    src_data = json.load(args.src)
>> -    dest_data = json.load(args.dest)
>> -    args.src.close()
>> -    args.dest.close()
>> +    with open(args.src, 'r', encoding='utf-8') as src_fh:
>> +        src_data = json.load(src_fh)
>> +    with open(args.dest, 'r', encoding='utf-8') as dst_fh:
>> +        dest_data = json.load(dst_fh)
> 
> This could be
> 
>   src_data = json.load(args.src.read_text('utf-8'))
>   dest_data = json.load(args.dest.read_text('utf-8'))

Does not work, looks like the load() function cannot deal with a string:

$ scripts/vmstate-static-checker.py -s 
tests/data/vmstate-static-checker/dump1.json -d 
tests/data/vmstate-static-checker/dump2.json
Traceback (most recent call last):
   File "../scripts/vmstate-static-checker.py", line 439, in <module>
     sys.exit(main())
              ~~~~^^
   File "../scripts/vmstate-static-checker.py", line 397, in main
     src_data = json.load(args.src.read_text('utf-8'))
   File "/usr/lib64/python3.13/json/__init__.py", line 293, in load
     return loads(fp.read(),
                  ^^^^^^^
AttributeError: 'str' object has no attribute 'read'

  Thomas


Re: [PATCH] scripts/vmstate-static-checker: Fix deprecation warnings with latest argparse
Posted by Daniel P. Berrangé 2 weeks, 1 day ago
On Thu, Oct 30, 2025 at 10:42:20AM +0100, Thomas Huth wrote:
> On 30/10/2025 10.33, Daniel P. Berrangé wrote:
> > On Thu, Oct 30, 2025 at 10:26:38AM +0100, Thomas Huth wrote:
> > > From: Thomas Huth <thuth@redhat.com>
> > > 
> > > The argparse.FileType() type has been deprecated in the latest argparse
> > > version (e.g. the one from Fedora 43), now causing the test_bad_vmstate
> > > functional test to fail since there are unexpected strings in the output.
> > > Change the script to use pathlib.Path instead to fix the test_bad_vmstate
> > > test and to be prepared for the future when the deprecated FileType gets
> > > removed completely.
> > > 
> > > Reported-by: Daniel P. Berrangé <berrange@redhat.com>
> > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > > ---
> > >   scripts/vmstate-static-checker.py | 13 +++++++------
> > >   1 file changed, 7 insertions(+), 6 deletions(-)
> > 
> > AFAICT, using pathlib.Path should work going back to any old python
> > versions we would need.
> 
> According to https://docs.python.org/3/library/pathlib.html it has been
> added with Python 3.4, and we require at least 3.9 nowadays, so yes, this
> should be fine.
> 
> > > @@ -393,10 +394,10 @@ def main():
> > >                           help='reverse the direction')
> > >       args = parser.parse_args()
> > > -    src_data = json.load(args.src)
> > > -    dest_data = json.load(args.dest)
> > > -    args.src.close()
> > > -    args.dest.close()
> > > +    with open(args.src, 'r', encoding='utf-8') as src_fh:
> > > +        src_data = json.load(src_fh)
> > > +    with open(args.dest, 'r', encoding='utf-8') as dst_fh:
> > > +        dest_data = json.load(dst_fh)
> > 
> > This could be
> > 
> >   src_data = json.load(args.src.read_text('utf-8'))
> >   dest_data = json.load(args.dest.read_text('utf-8'))
> 
> Does not work, looks like the load() function cannot deal with a string:
> 
> $ scripts/vmstate-static-checker.py -s
> tests/data/vmstate-static-checker/dump1.json -d
> tests/data/vmstate-static-checker/dump2.json
> Traceback (most recent call last):
>   File "../scripts/vmstate-static-checker.py", line 439, in <module>
>     sys.exit(main())
>              ~~~~^^
>   File "../scripts/vmstate-static-checker.py", line 397, in main
>     src_data = json.load(args.src.read_text('utf-8'))
>   File "/usr/lib64/python3.13/json/__init__.py", line 293, in load
>     return loads(fp.read(),
>                  ^^^^^^^
> AttributeError: 'str' object has no attribute 'read'

Sorry, I typod - it should be 'json.loads'  - note the trailing
's' for "string"

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH] scripts/vmstate-static-checker: Fix deprecation warnings with latest argparse
Posted by Thomas Huth 2 weeks, 1 day ago
On 30/10/2025 10.42, Thomas Huth wrote:
> On 30/10/2025 10.33, Daniel P. Berrangé wrote:
>> On Thu, Oct 30, 2025 at 10:26:38AM +0100, Thomas Huth wrote:
>>> From: Thomas Huth <thuth@redhat.com>
>>>
>>> The argparse.FileType() type has been deprecated in the latest argparse
>>> version (e.g. the one from Fedora 43), now causing the test_bad_vmstate
>>> functional test to fail since there are unexpected strings in the output.
>>> Change the script to use pathlib.Path instead to fix the test_bad_vmstate
>>> test and to be prepared for the future when the deprecated FileType gets
>>> removed completely.
...
>>> @@ -393,10 +394,10 @@ def main():
>>>                           help='reverse the direction')
>>>       args = parser.parse_args()
>>> -    src_data = json.load(args.src)
>>> -    dest_data = json.load(args.dest)
>>> -    args.src.close()
>>> -    args.dest.close()
>>> +    with open(args.src, 'r', encoding='utf-8') as src_fh:
>>> +        src_data = json.load(src_fh)
>>> +    with open(args.dest, 'r', encoding='utf-8') as dst_fh:
>>> +        dest_data = json.load(dst_fh)
>>
>> This could be
>>
>>   src_data = json.load(args.src.read_text('utf-8'))
>>   dest_data = json.load(args.dest.read_text('utf-8'))
> 
> Does not work, looks like the load() function cannot deal with a string:
> 
> $ scripts/vmstate-static-checker.py -s tests/data/vmstate-static-checker/ 
> dump1.json -d tests/data/vmstate-static-checker/dump2.json
> Traceback (most recent call last):
>    File "../scripts/vmstate-static-checker.py", line 439, in <module>
>      sys.exit(main())
>               ~~~~^^
>    File "../scripts/vmstate-static-checker.py", line 397, in main
>      src_data = json.load(args.src.read_text('utf-8'))
>    File "/usr/lib64/python3.13/json/__init__.py", line 293, in load
>      return loads(fp.read(),
>                   ^^^^^^^
> AttributeError: 'str' object has no attribute 'read'

Ok, there also seems to be a json.loads() function (with "s" at the end) 
that seems to do the job ... but IMHO it would be better to continue using 
the load() function here to let the json parser decide how to read the file.

  Thomas


Re: [PATCH] scripts/vmstate-static-checker: Fix deprecation warnings with latest argparse
Posted by Daniel P. Berrangé 2 weeks, 1 day ago
On Thu, Oct 30, 2025 at 10:45:32AM +0100, Thomas Huth wrote:
> On 30/10/2025 10.42, Thomas Huth wrote:
> > On 30/10/2025 10.33, Daniel P. Berrangé wrote:
> > > On Thu, Oct 30, 2025 at 10:26:38AM +0100, Thomas Huth wrote:
> > > > From: Thomas Huth <thuth@redhat.com>
> > > > 
> > > > The argparse.FileType() type has been deprecated in the latest argparse
> > > > version (e.g. the one from Fedora 43), now causing the test_bad_vmstate
> > > > functional test to fail since there are unexpected strings in the output.
> > > > Change the script to use pathlib.Path instead to fix the test_bad_vmstate
> > > > test and to be prepared for the future when the deprecated FileType gets
> > > > removed completely.
> ...
> > > > @@ -393,10 +394,10 @@ def main():
> > > >                           help='reverse the direction')
> > > >       args = parser.parse_args()
> > > > -    src_data = json.load(args.src)
> > > > -    dest_data = json.load(args.dest)
> > > > -    args.src.close()
> > > > -    args.dest.close()
> > > > +    with open(args.src, 'r', encoding='utf-8') as src_fh:
> > > > +        src_data = json.load(src_fh)
> > > > +    with open(args.dest, 'r', encoding='utf-8') as dst_fh:
> > > > +        dest_data = json.load(dst_fh)
> > > 
> > > This could be
> > > 
> > >   src_data = json.load(args.src.read_text('utf-8'))
> > >   dest_data = json.load(args.dest.read_text('utf-8'))
> > 
> > Does not work, looks like the load() function cannot deal with a string:
> > 
> > $ scripts/vmstate-static-checker.py -s
> > tests/data/vmstate-static-checker/ dump1.json -d
> > tests/data/vmstate-static-checker/dump2.json
> > Traceback (most recent call last):
> >    File "../scripts/vmstate-static-checker.py", line 439, in <module>
> >      sys.exit(main())
> >               ~~~~^^
> >    File "../scripts/vmstate-static-checker.py", line 397, in main
> >      src_data = json.load(args.src.read_text('utf-8'))
> >    File "/usr/lib64/python3.13/json/__init__.py", line 293, in load
> >      return loads(fp.read(),
> >                   ^^^^^^^
> > AttributeError: 'str' object has no attribute 'read'
> 
> Ok, there also seems to be a json.loads() function (with "s" at the end)
> that seems to do the job ... but IMHO it would be better to continue using
> the load() function here to let the json parser decide how to read the file.

ok, I don't mind that much

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH] scripts/vmstate-static-checker: Fix deprecation warnings with latest argparse
Posted by Thomas Huth 2 weeks, 1 day ago
On 30/10/2025 11.01, Daniel P. Berrangé wrote:
> On Thu, Oct 30, 2025 at 10:45:32AM +0100, Thomas Huth wrote:
>> On 30/10/2025 10.42, Thomas Huth wrote:
>>> On 30/10/2025 10.33, Daniel P. Berrangé wrote:
>>>> On Thu, Oct 30, 2025 at 10:26:38AM +0100, Thomas Huth wrote:
>>>>> From: Thomas Huth <thuth@redhat.com>
>>>>>
>>>>> The argparse.FileType() type has been deprecated in the latest argparse
>>>>> version (e.g. the one from Fedora 43), now causing the test_bad_vmstate
>>>>> functional test to fail since there are unexpected strings in the output.
>>>>> Change the script to use pathlib.Path instead to fix the test_bad_vmstate
>>>>> test and to be prepared for the future when the deprecated FileType gets
>>>>> removed completely.
>> ...
>>>>> @@ -393,10 +394,10 @@ def main():
>>>>>                            help='reverse the direction')
>>>>>        args = parser.parse_args()
>>>>> -    src_data = json.load(args.src)
>>>>> -    dest_data = json.load(args.dest)
>>>>> -    args.src.close()
>>>>> -    args.dest.close()
>>>>> +    with open(args.src, 'r', encoding='utf-8') as src_fh:
>>>>> +        src_data = json.load(src_fh)
>>>>> +    with open(args.dest, 'r', encoding='utf-8') as dst_fh:
>>>>> +        dest_data = json.load(dst_fh)
>>>>
>>>> This could be
>>>>
>>>>    src_data = json.load(args.src.read_text('utf-8'))
>>>>    dest_data = json.load(args.dest.read_text('utf-8'))
>>>
>>> Does not work, looks like the load() function cannot deal with a string:
>>>
>>> $ scripts/vmstate-static-checker.py -s
>>> tests/data/vmstate-static-checker/ dump1.json -d
>>> tests/data/vmstate-static-checker/dump2.json
>>> Traceback (most recent call last):
>>>     File "../scripts/vmstate-static-checker.py", line 439, in <module>
>>>       sys.exit(main())
>>>                ~~~~^^
>>>     File "../scripts/vmstate-static-checker.py", line 397, in main
>>>       src_data = json.load(args.src.read_text('utf-8'))
>>>     File "/usr/lib64/python3.13/json/__init__.py", line 293, in load
>>>       return loads(fp.read(),
>>>                    ^^^^^^^
>>> AttributeError: 'str' object has no attribute 'read'
>>
>> Ok, there also seems to be a json.loads() function (with "s" at the end)
>> that seems to do the job ... but IMHO it would be better to continue using
>> the load() function here to let the json parser decide how to read the file.
> 
> ok, I don't mind that much

I'm basically thinking of situations where you have a big dump file: When 
using loads(), you have to completely read the file into memory first, maybe 
using a lot of memory. With load(), the parser could digest it bit by bit 
instead, and e.g. bail out early if there is a bug in the json format of the 
file. Well, it likely doesn't matter too much for the file sizes that we use 
in QEMU, but still... I think it's nicer with load().

> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Thanks!

  Thomas