Re: [PATCH v1 06/14] tests/vm: Add logging of console to file.

[Robert Foley]

On Fri, 7 Feb 2020 at 17:20, Robert Foley <address@hidden> wrote:
>
> On Fri, 7 Feb 2020 at 12:12, Alex Bennée <address@hidden> wrote:
> > > +
> > > +    def join(self, timeout=None):
> > > +        """Time to destroy the thread.
> > > +           Clear the event to stop the thread, and wait for
> > > +           it to complete."""
> > > +        self.alive.clear()
> > > +        threading.Thread.join(self, timeout)
> > > +        self.log_file.close()
> >
> > I'm note sure about this - introducing threading into Python seems very
> > un-pythonic. I wonder if the python experts have any view on a better
> > way to achieve what we want which I think is:
> >
> >   - a buffer that properly drains output from QEMU
> >   - which can optionally be serialised onto disk for logging
> >
> > What else are we trying to achieve here?
>
> I think that covers what we are trying to achieve here.
> The logging to file is a nice to have, but
> the draining of the output from QEMU is the main objective here.
> We will do a bit more research here to seek out a cleaner way to achieve this,
> but certainly we would also be interested if any python experts have a
> view on a cleaner approach.
>
We have a few more ideas on how to make this a bit cleaner.

We could create a new class "ConsoleSocket", based on asyncore.dispatcher.
The asyncore python library allows for overriding certain functionality of
an underlying socket.
We could override the handle_read() method in order to get a callback
when data is available to be read from the console.  This is much cleaner
than us simply waiting with a read() on the socket in a thread.
Note that even asyncore requires a thread around asyncore.loop(), but we would
only be responsible for start/stop of the thread and the thread body
is defined for us (literally just asyncore.loop()).
The thread starting/stopping would be completely transparent
to the users of this new ConsoleSocket.  The thread would be created on
initialization of ConsoleSocket, and destroyed upon close() of the socket.

As the console bytes get consumed we can dump them to an in memory stream,
rather than dumping them to a file right away.
We could perhaps use an io.StringIO text stream.  This stream will get
the bytes of the console and the test will consume the bytes directly
from this stream.
In addition, we could optionally dump the bytes to a file as well.

How does this sound?  Just looking for a bit of feedback before we
head in this direction.

Thanks & Regards,
-Rob