On 10/1/19 3:46 PM, Max Reitz wrote:
> First of all: Sorry.
>
Thank you for finding the time to do it.
>
> Second:
>
> Based-on: My block branch
> (https://github.com/XanClic/qemu.git block)
>
> Based-on: 20190917234549.22910-1-jsnow@redhat.com
> (“iotests: use python logging”)
>
> Based-on: 20190927094242.11152-1-mreitz@redhat.com
> (“iotests: Allow ./check -o data_file”)
>
> Based-on: 20190917092004.999-1-mreitz@redhat.com
> (“iotests: Selfish patches”)
>
> Based-on: 20191001174827.11081-1-mreitz@redhat.com
> (“block: Skip COR for inactive nodes”)
>
>
> OK, now:
>
> Hi,
>
> My recent series “iotests: Allow ./check -o data_file” enabled our bash
> tests to interpret the data_file qcow2 option. It didn’t do anything
> for Python tests because those currently completely ignore all image
> format options.
>
> This is where it gets hairy. To do so, we need two things: First of
> all, whatever way Python tests use to create images needs to interpret
> $IMGOPTS. Second, when deleting image files, they must not use a plain
> os.remove(), but a special function that will clean up data files, too.
>
> The heap of patches in this series comes from making the Python tests
> use these new functions.
>
> Most Python tests just run qemu-img through a helper function that does
> not care about the exact subcommand to create images. I could add
> $IMGOPTS support to it, but that doesn’t feel quite right to me, and it
> wouldn’t reduce the patch count because we still need a special removal
> function.
>
>
> This series is structured as follows:
> - Patches 1 through 7 add support to handle image files differently from
> other files (consider $IMGOPTS when creating them, consider data files
> when deleting them, separate ImagePaths from FilePaths, and so on)
OK, that makes sense. I suppose we've been playing a bit fast and loose
with these such things.
>
> - Patches 8 and 9 add two filters we’ll need in the next range:
>
> - Patches 10 through 13 address some issues in a handful of tests that
> just need to be changed a little so they can overall work with some
> format options
>
> - Patch 14 makes all tests pass unsupported_imgopts where there are
> options that they cannot support.
>
> - Patches 15 through 65 make all Python tests use the new functions
> introduced in the first 7 patches so they no longer ignore $IMGOPTS.
>
> I felt like this is much better than munching everything together into
> a single big commit (better to rebase, better to review), and I don’t
> really like ideas like “Just do five patches that each address ten
> iotests”.
>
This is the right approach, for the exact reasons you specify.
> But I’m still very much open to suggestions on how to combine these
> many small patches to reduce the overall patch count.
>
You could group them by release windows, if you really wanted to;
- [...etc...]
- Update iotests added for 3.2
- Update iotests added for 4.0
- Update iotests added for 4.1
- Individual patches thereafter
But maybe that doesn't really solve anything for anyone. If you didn't
find a more obvious grouping for these, I'd just leave it alone. I'll
get to reviewing them.
> - Patch 66 ensures that Python tests always use the new function to
> create test images so they won’t bypass $IMGOPTS.
>
> - Patch 67 cleans up. qemu_img_log() is only used for image creation,
> and I don’t see the point in that. The output is predictable and it
> is very unlikely to fail. We can see in the bash tests that regularly
> we basically just filter everything from it anyway.
> (So this series replaces log(qemu_img_pipe()) instances by asserting
> that image creation did not fail.)
> ((qemu_img_create() obviously no longer has any use after this
> series.))
>
>
> After this series, running the iotests with -o compat=0.10,
> -o refcount_bits=1, and -o 'data_file=$TEST_IMG.data_file' does
> something sensible even for the Python tests, and it passes.
>
No minor accomplishment.
I'll make sure to review at least 1-14, but not before Friday.
>
> Max Reitz (67):
> iotests.py: Read $IMGOPTS
> iotests.py: Add @skip_for_imgopts()
> iotests.py: Add unsupported_imgopts
> iotests.py: create_test_image, remove_test_image
> iotests.py: Add ImagePaths
> iotests.py: Add image_path()
> iotests.py: Filter data_file in filter_img_info
> iotests.py: Add filter_json_filename()
> iotests.py: Add @hide_fields to img_info_log
> iotests/169: Skip persistent cases for compat=0.10
> iotests/224: Filter json:{} from commit command
> iotests/228: Filter json:{} filenames
> iotests/242: Hide refcount bit information
> iotests: Use unsupported_imgopts in Python tests
> iotests/030: Honor $IMGOPTS
> iotests/040: Honor $IMGOPTS
> iotests/041: Honor $IMGOPTS
> iotests/044: Honor $IMGOPTS
> iotests/045: Honor $IMGOPTS
> iotests/055: Honor $IMGOPTS
> iotests/056: Honor $IMGOPTS
> iotests/057: Honor $IMGOPTS
> iotests/065: Honor $IMGOPTS
> iotests/096: Honor $IMGOPTS
> iotests/118: Honor $IMGOPTS
> iotests/124: Honor $IMGOPTS
> iotests/129: Honor $IMGOPTS
> iotests/132: Honor $IMGOPTS
> iotests/139: Honor $IMGOPTS
> iotests/147: Honor $IMGOPTS
> iotests/148: Honor $IMGOPTS
> iotests/151: Honor $IMGOPTS
> iotests/152: Honor $IMGOPTS
> iotests/155: Honor $IMGOPTS
> iotests/163: Honor $IMGOPTS
> iotests/165: Honor $IMGOPTS
> iotests/169: Honor $IMGOPTS
> iotests/194: Honor $IMGOPTS
> iotests/196: Honor $IMGOPTS
> iotests/199: Honor $IMGOPTS
> iotests/202: Honor $IMGOPTS
> iotests/203: Honor $IMGOPTS
> iotests/205: Honor $IMGOPTS
> iotests/208: Honor $IMGOPTS
> iotests/208: Honor $IMGOPTS
> iotests/216: Honor $IMGOPTS
> iotests/218: Honor $IMGOPTS
> iotests/219: Honor $IMGOPTS
> iotests/222: Honor $IMGOPTS
> iotests/224: Honor $IMGOPTS
> iotests/228: Honor $IMGOPTS
> iotests/234: Honor $IMGOPTS
> iotests/235: Honor $IMGOPTS
> iotests/236: Honor $IMGOPTS
> iotests/237: Honor $IMGOPTS
> iotests/242: Honor $IMGOPTS
> iotests/245: Honor $IMGOPTS
> iotests/246: Honor $IMGOPTS
> iotests/248: Honor $IMGOPTS
> iotests/254: Honor $IMGOPTS
> iotests/255: Honor $IMGOPTS
> iotests/256: Honor $IMGOPTS
> iotests/257: Honor $IMGOPTS
> iotests/258: Honor $IMGOPTS
> iotests/262: Honor $IMGOPTS
> iotests.py: Forbid qemu_img*('create', ...)
> iotests.py: Drop qemu_img_log(), qemu_img_create()
>
> tests/qemu-iotests/030 | 69 ++++++------
> tests/qemu-iotests/040 | 42 ++++----
> tests/qemu-iotests/041 | 108 +++++++++----------
> tests/qemu-iotests/044 | 11 +-
> tests/qemu-iotests/045 | 26 ++---
> tests/qemu-iotests/055 | 41 +++----
> tests/qemu-iotests/056 | 30 +++---
> tests/qemu-iotests/057 | 10 +-
> tests/qemu-iotests/065 | 21 ++--
> tests/qemu-iotests/096 | 5 +-
> tests/qemu-iotests/118 | 26 ++---
> tests/qemu-iotests/124 | 29 +++--
> tests/qemu-iotests/129 | 11 +-
> tests/qemu-iotests/132 | 6 +-
> tests/qemu-iotests/139 | 15 ++-
> tests/qemu-iotests/147 | 11 +-
> tests/qemu-iotests/148 | 5 +-
> tests/qemu-iotests/151 | 10 +-
> tests/qemu-iotests/152 | 6 +-
> tests/qemu-iotests/155 | 29 +++--
> tests/qemu-iotests/163 | 29 ++---
> tests/qemu-iotests/165 | 10 +-
> tests/qemu-iotests/169 | 23 ++--
> tests/qemu-iotests/194 | 9 +-
> tests/qemu-iotests/196 | 10 +-
> tests/qemu-iotests/199 | 10 +-
> tests/qemu-iotests/202 | 9 +-
> tests/qemu-iotests/203 | 9 +-
> tests/qemu-iotests/205 | 7 +-
> tests/qemu-iotests/206 | 5 +-
> tests/qemu-iotests/208 | 5 +-
> tests/qemu-iotests/209 | 9 +-
> tests/qemu-iotests/216 | 11 +-
> tests/qemu-iotests/218 | 6 +-
> tests/qemu-iotests/219 | 5 +-
> tests/qemu-iotests/222 | 13 +--
> tests/qemu-iotests/224 | 33 +++---
> tests/qemu-iotests/224.out | 4 +-
> tests/qemu-iotests/228 | 25 ++---
> tests/qemu-iotests/228.out | 8 +-
> tests/qemu-iotests/234 | 9 +-
> tests/qemu-iotests/235 | 7 +-
> tests/qemu-iotests/236 | 6 +-
> tests/qemu-iotests/237 | 15 +--
> tests/qemu-iotests/237.out | 6 --
> tests/qemu-iotests/242 | 21 ++--
> tests/qemu-iotests/242.out | 5 -
> tests/qemu-iotests/245 | 21 ++--
> tests/qemu-iotests/246 | 11 +-
> tests/qemu-iotests/248 | 14 ++-
> tests/qemu-iotests/254 | 10 +-
> tests/qemu-iotests/255 | 20 ++--
> tests/qemu-iotests/255.out | 8 --
> tests/qemu-iotests/256 | 10 +-
> tests/qemu-iotests/257 | 18 ++--
> tests/qemu-iotests/258 | 16 +--
> tests/qemu-iotests/262 | 5 +-
> tests/qemu-iotests/iotests.py | 197 +++++++++++++++++++++++++++-------
> 58 files changed, 654 insertions(+), 496 deletions(-)
>