Re: [Qemu-block] [Qemu-devel] [PATCH v5 03/10] qemu-iotests: automatically clean up bash protocol servers

[Paolo Bonzini]

On 17/10/2017 18:31, Jeff Cody wrote:
>              )
>          else
> +            # Do this in a sub-shell, so we are operating on the right
> +            # TEST_DIR / QEMU_TEST_DIR
>              (
>              export TEST_DIR=$TEST_DIR_SEQ
>              cd "$source_iotests";

Where is the missing ")"?

> @@ -837,6 +841,15 @@ do
>              fi
>          fi
>  
> +        # Do this in a sub-shell, so we are operating on the right
> +        # TEST_DIR / QEMU_TEST_DIR
> +        (
> +        export TEST_DIR=$TEST_DIR_SEQ
> +        . "$source_iotests/common.config"
> +        . "$source_iotests/common.rc"
> +
> +        _cleanup_protocols

"check" wasn't including common.rc before this patch, and most of the
content of that file doesn't apply to "check".  So if we want to move
cleanup code to "check" we should remove it from common.rc too.

In general, I think we should strive to have a better separation between
tests and harness.  This for example could let us write a different
harness for the same tests and integrate the tests better into Avocado.
Hence I see one advantage and one disadvantage in your series:

* by adding more functionality to "check", it shows that the current
separation may fail with a more sophisticated harness (such as the one
you are creating here)

* it adds a lot more knowledge of QEMU (especially protocols) in
"check", but there is still some unbalance: tests create the images and
the protocol servers, but the harness cleans it up.  The visible result
of this unbalance is for example how multi-process protocol tests can
fail when multiple tests try to bind the same address.

I think it's the right direction, but it feels like it's not there
yet...  Sorry---I know this is not very constructive, but I hope it
helps anyway.

Maybe we should actually rewrite "check" in Python.  That would force us
to think more about the design.

Paolo