On Mon, Oct 12, 2009 at 09:07:45AM -0400, Michael Goldish wrote:
> You're right, currently the sessions must be closed explicitly.
> This is due to the fact that both qemu and ssh/telnet are handled by the
> same code, and qemu has to keep running in the background if we want to
> pass it from one test to another.
>
> To deal with this, first, we should use try..finally blocks to close all
> sessions in tests. As far as I know all existing tests (or at least most
> of them) already do this.
>
> Second, we can add a destructor function to kvm_shell_session that will
> close the session automatically when it's no longer referenced. At the
> moment this won't work because there's a thread running in the background
> tracking output from the session, but this thread is usually not needed
> for ssh/telnet (it's needed mainly for qemu), so we can selectively get
> rid of it and allow the reference count to drop to zero when the test exits,
> thus allowing the destructor to be called.
>
> I'll think of a way to do the second thing, and if it works, maybe we won't
> need the first. But for now every test should close its sessions explicitly.
>
> BTW, I'm not sure I understand why cleaning up the sessions should be
> exhausting in the case you presented. You can just wrap everything in one
> big try..finally block:
>
> session = ...
>
> try:
> try:
> except:
> try:
> except:
> ...
> finally:
> session.close()
>
Thanks for your explanation.
It is just boring and error-prone to add lots of
'(dst|src|tmp|etc)*sesson.close()' to our code (the internal version)
into different files and big number of functions. and some of the
'sessions' are in the try...except blocks, and some are not.
We have to make sure where we started the sessions and to close all of
them when they are not needed any longer. I feel it's a little weird
that we have to do the garbage-collection-like work while using python.
so, since this is a known issue, or precisely, limitation of 'ease of
use', I'm looking forward to your impovement, and I think, we will
also try to work it out at the same time.
Thanks again for your help.
Cao, Chen
2009-10-13
> ----- Original Message -----
> From: "Chen Cao" <k...@redhat.com>
> To: "Michael Goldish" <mgold...@redhat.com>
> Cc: autot...@test.kernel.org, kvm@vger.kernel.org
> Sent: Monday, October 12, 2009 8:55:59 AM (GMT+0200) Auto-Detected
> Subject: Re: [KVM-AUTOTEST,01/17] Add new module kvm_subprocess
>
>
> Hi, Michael,
>
> I found that if the sessions initialized using kvm_subprcoess are not closed,
> the processes will never exit, and /tmp/kvm_spawn will be filled with the
> temporary files.
>
> And we can find in the code,
> # kvm_subprocess.py
> ...
> # Read from child and write to files/pipes
> while True:
> check_termination = False
> # Make a list of reader pipes whose buffers are not empty
> fds = [fd for (i, fd) in enumerate(reader_fds) if buffers[i]]
> # Wait until there's something to do
> r, w, x = select.select([shell_fd, inpipe_fd], fds, [], 0.5)
> # If a reader pipe is ready for writing --
> for (i, fd) in enumerate(reader_fds):
> if fd in w:
> bytes_written = os.write(fd, buffers[i])
> buffers[i] = buffers[i][bytes_written:]
> # If there's data to read from the child process --
> if shell_fd in r:
> try:
> data = os.read(shell_fd, 16384)
> except OSError:
> data = ""
> if not data:
> check_termination = True
> # Remove carriage returns from the data -- they often cause
> # trouble and are normally not needed
> data = data.replace("\r", "")
> output_file.write(data)
> output_file.flush()
> for i in range(len(readers)):
> buffers[i] += data
> # If os.read() raised an exception or there was nothing to read --
> if check_termination or shell_fd not in r:
> pid, status = os.waitpid(shell_pid, os.WNOHANG)
> if pid:
> status = os.WEXITSTATUS(status)
> break
> # If there's data to read from the client --
> if inpipe_fd in r:
> data = os.read(inpipe_fd, 1024)
> os.write(shell_fd, data)
> ...
>
> that if session.close() is not called, we will loop in the 'while' forever.
>
> So, user have to make sure that unnecessary sessions are all killed,
> otherwise, running some testcase(s) for huge number of times will suck
> out all the system resource, which I think is very inconvenient.
> Especially when we have to take care of many exceptions that may be raised
> by our program.
>
> e.g.
>
> ...
> session = kvm_test_utils.wait_for_login(vm)
> ...
> session2 = kvm_test_utils.wait_for_login(vm_x)
> ...
> try:
> ...
> except ...:
> ...
> ...
> (other code may raise exceptions)
> ...
> try:
> ...
> except ...:
> ...
> ...
> try:
> ...
> except ...:
> ...
> ...
>
> cleaning up the sessions will be exhausting here.
>
> Do we have a good (or better) way to handle this?
> Thanks.
>
>
> Regards,
>
> Cao, Chen
> 2009-10-12
>
> On Mon, Jul 20, 2009 at 12:07 PM, Michael Goldish<mgold...@redhat.com>
> wrote:
> > This module is intended to be used for controlling all child
> > processes in KVM
> > tests: both QEMU processes and SSH/SCP/Telnet processes. Processes
> > started with
> > this module keep running and can be interacted with even after the
> > parent
> > process exits.
> >
> > The current run_bg() utility tracks a child process as long as the
> > parent
> > process is running. When the parent process exits, the tracking
> > thread
> > terminates and cannot resume when needed.
> >
> > Currently SSH/SCP/Telnet communication is handled by
> > kvm_utils.kvm_spawn, which
> > does not allow the child process to run after the parent process
> > exits. Thus,
> > open SSH/SCP/Telnet sessions cannot be reused by tests following the
> > one in
> > which they are opened.
> >
> > The new module provides a solution to these two problems, and also
> > saves some
> > code by reusing common code required both for QEMU processes and
> > SSH/SCP/Telnet
> > processes.
> >
> > Signed-off-by: Michael Goldish <mgold...@redhat.com>
> > ---
> > client/tests/kvm/kvm_subprocess.py | 1146
> > ++++++++++++++++++++++++++++++++++++
> > 1 files changed, 1146 insertions(+), 0 deletions(-)
> > create mode 100644 client/tests/kvm/kvm_subprocess.py
> >
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
