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

[Jeff Cody]

On Wed, Oct 18, 2017 at 04:46:20PM +0200, Paolo Bonzini wrote:
> 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 ")"?

It's part of the diff context, not a change itself... it's still there, just
not shown in the patch.

> 
> > @@ -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.
> 

I think writing a more sophisticated harness in another language, such as
Python, has merit.  But to clarify, do you mean that as a 'nack' to this
series, or as something to be done later, after this series?  If this series
improves the existing harness, I don't see why to exclude it because a new
re-write could be superior (which I don't dispute).