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

[Robert Foley]

On Fri, 7 Feb 2020 at 12:12, Alex Bennée <address@hidden> wrote:
> Robert Foley <address@hidden> writes:
>
> > This adds logging of the char device used by the console
> > to a file.  The basevm.py then uses this file to read
> > chars from the console.
> > One reason to add this is to aid with debugging.
>
> I can certainly see an argument for saving the install log.
>
> > But another is because there is an issue where the QEMU
> > might hang if the characters from the character device
> > are not consumed by the script.
>
> I'm a little confused by this. Outputting to a file and then parsing the
> file seems a bit more janky than injesting the output in the script and
> then logging it.
>
> Is this to work around the hang because the socket buffers fill up and
> aren't drained?

Yes, exactly.  This is to work around the hang we are seeing when we
try to use these new VMs.

> > +    console_logfile = "console.log"
>
> We should probably dump the log somewhere other than cwd. Given we cache
> stuff in ~/.cache/qemu-vm maybe something of the form:
>
>   ~/.cache/qemu-vm/ubuntu.aarch64.install.log
>
> ?

Good point, we will locate the log file there.

> > +            elapsed_sec = time.time() - start_time
> > +            if elapsed_sec > self._console_timeout_sec:
> > +                raise ConsoleTimeoutException
> > +        return data.encode('latin1')
> > +
>
> Is latin1 really the best choice here? I would expect things to be utf-8 
> clean.

We were trying to follow the existing code, which is using latin1.
For example, console_wait() or console_consume() are using latin1.
However on further inspection we see that console_send() is using utf-8.
We will look at changing these latin1 cases to be utf-8.
I also found a case in get_qemu_version() we will change to utf-8 also.

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

Thanks & Regards,
-Rob
>
> --
> Alex Bennée