From: Robert Foley <robert.foley@linaro.org>
To: "Alex Bennée" <alex.bennee@linaro.org>
Cc: "Peter Puhov" <peter.puhov@linaro.org>,
"Cleber Rosa" <crosa@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>,
"QEMU Developers" <qemu-devel@nongnu.org>,
"Eduardo Habkost" <ehabkost@redhat.com>
Subject: Re: [PATCH v7 12/12] tests/vm: Add workaround to consume console
Date: Fri, 22 May 2020 16:44:43 -0400 [thread overview]
Message-ID: <CAEyhzFt=bwj_-TU2Mj3ts6HwfTaC0Rj8e6UoOJcFCjPqHU9LqQ@mail.gmail.com> (raw)
In-Reply-To: <87zha0c4nw.fsf@linaro.org>
On Fri, 22 May 2020 at 12:31, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Robert Foley <robert.foley@linaro.org> writes:
>
> I think you need to look at adding:
>
> [sendemail]
> cccmd = scripts/get_maintainer.pl --nogit-fallback
>
> to your .git/config to ensure maintainers get pinged when you touch
> their subsystems. Eduardo and Cleber CC'd
Thanks for pointing this out! We will definitely add it and use it.
> > The ConsoleSocket object provides a socket interface
> > which will consume all arriving characters on the
> > socket, but will provide those chars via recv() as
> > would a regular socket.
> > This is a workaround we found was needed since
> > there is a known issue where QEMU will hang waiting
> > for console characters to be consumed.
> > We also add the option of logging the console to a file.
> >
> > Signed-off-by: Robert Foley <robert.foley@linaro.org>
> > Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
> > ---
> > python/qemu/console_socket.py | 162 ++++++++++++++++++++++++++++++++++
> > python/qemu/machine.py | 23 ++++-
> > tests/vm/Makefile.include | 4 +
> > tests/vm/basevm.py | 19 +++-
> > 4 files changed, 202 insertions(+), 6 deletions(-)
> > create mode 100644 python/qemu/console_socket.py
> >
> > diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
<snip>
> > +import traceback
>
> Left over debug?
This is getting used here in a try except in handle_read, to display
the exception information.
<snip>
> > + def handle_read(self):
> > + """process arriving characters into in memory _buffer"""
> > + try:
> > + data = asyncore.dispatcher.recv(self, 1)
> > + # latin1 is needed since there are some chars
> > + # we are receiving that cannot be encoded to utf-8
> > + # such as 0xe2, 0x80, 0xA6.
> > + string = data.decode("latin1")
> > + except:
> > + print("Exception seen.")
> > + traceback.print_exc()
> > + return
> > + if self._logfile:
> > + self._logfile.write("{}".format(string))
> > + self._logfile.flush()
> > + for c in string:
> > + self._buffer.append(c)
<snip>
> > +if __name__ == '__main__':
>
> If the module is meant to be executable then you need to +x the file.
> However since 8f8fd9edba I think everything is meant to be doing things
> the pythonic way as a proper module. I'm not sure where unit tests for
> the modules are meant to sit in this case.
That is a good point. I see the other modules at this level do not have
tests like this, so I am going to remove this for now, as I think it adds
limited value at this point.
>
> > + # Brief test to exercise the above code.
> > + # The ConsoleSocket will ship some data to the server,
> > + # the server will echo it back and the client will echo what it received.
> > +
> > + # First remove the socket.
> > + address = "./test_console_socket"
> > + if os.path.exists(address):
> > + os.unlink(address)
> > +
> > + # Create the server side.
> > + server_socket = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
> > + server_socket.bind(address)
> > + server_socket.listen(1)
> > +
> > + # Create the object we are trying to test.
> > + console_socket = ConsoleSocket(address, file="./logfile.txt")
> > +
> > + # Generate some data and ship it over the socket.
> > + send_data = ""
> > + for i in range(10):
> > + send_data += "this is a test message {}\n".format(i)
> > + console_socket.send(send_data.encode('latin1'))
> > + connection, client_address = server_socket.accept()
> > +
> > + # Process the data on the server and ship it back.
> > + data = connection.recv(len(send_data))
> > + print("server received: {}".format(data))
> > + print("server: sending data back to the client")
> > + connection.sendall(data)
> > +
> > + # Client receives teh bytes and displays them.
>
> s/teh/the/
>
> > + print("client: receiving bytes")
> > + bytes = console_socket.recv(len(data))
> > + recv_data = bytes.decode('latin1')
> > + print("client received: {}".format(recv_data))
> > + assert(recv_data == send_data)
> > + # Close console connection first, then close server.
> > + console_socket.close()
> > + connection.close()
> > + server_socket.close()
> > + print("test successful.")
> > +
>
> I think in this case it might be worth splitting introducing the
> functionally into the python library from the actual usage of it in the
> wider machines.
OK, I'll split this out into a separate patch.
Thanks & Regards,
-Rob
>
> Otherwise it seems to work well enough for me. I'd like the proper
> python gurus to have a look over it though.
>
> Acked-by: Alex Bennée <alex.bennee@linaro.org>
>
> --
> Alex Bennée
prev parent reply other threads:[~2020-05-22 20:45 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-19 13:22 [PATCH v7 00/12] tests/vm: Add support for aarch64 VMs Robert Foley
2020-05-19 13:22 ` [PATCH v7 01/12] configure: add alternate binary for genisoimage Robert Foley
2020-05-19 13:22 ` [PATCH v7 02/12] tests/vm: pass --genisoimage to basevm script Robert Foley
2020-05-19 13:22 ` [PATCH v7 03/12] tests/vm: pass args through to BaseVM's __init__ Robert Foley
2020-05-20 21:49 ` Alex Bennée
2020-05-22 12:58 ` Robert Foley
2020-05-19 13:22 ` [PATCH v7 04/12] tests/vm: Add configuration to basevm.py Robert Foley
2020-05-22 14:22 ` Alex Bennée
2020-05-19 13:22 ` [PATCH v7 05/12] tests/vm: Added configuration file support Robert Foley
2020-05-22 14:26 ` Alex Bennée
2020-05-19 13:22 ` [PATCH v7 06/12] tests/vm: Pass --debug through for vm-boot-ssh Robert Foley
2020-05-22 14:29 ` Alex Bennée
2020-05-19 13:22 ` [PATCH v7 07/12] tests/vm: Add ability to select QEMU from current build Robert Foley
2020-05-22 14:40 ` Alex Bennée
2020-05-22 18:53 ` Robert Foley
2020-05-19 13:22 ` [PATCH v7 08/12] tests/vm: allow wait_ssh() to specify command Robert Foley
2020-05-19 13:22 ` [PATCH v7 09/12] tests/vm: Added a new script for ubuntu.aarch64 Robert Foley
2020-05-22 15:34 ` Alex Bennée
2020-05-22 19:24 ` Robert Foley
2020-05-19 13:22 ` [PATCH v7 10/12] tests/vm: Added a new script for centos.aarch64 Robert Foley
2020-05-22 15:59 ` Alex Bennée
2020-05-22 19:44 ` Robert Foley
2020-05-19 13:22 ` [PATCH v7 11/12] tests/vm: change scripts to use self._config Robert Foley
2020-05-19 13:22 ` [PATCH v7 12/12] tests/vm: Add workaround to consume console Robert Foley
2020-05-22 16:31 ` Alex Bennée
2020-05-22 20:44 ` Robert Foley [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAEyhzFt=bwj_-TU2Mj3ts6HwfTaC0Rj8e6UoOJcFCjPqHU9LqQ@mail.gmail.com' \
--to=robert.foley@linaro.org \
--cc=alex.bennee@linaro.org \
--cc=crosa@redhat.com \
--cc=ehabkost@redhat.com \
--cc=peter.puhov@linaro.org \
--cc=philmd@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).