qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Thomas Huth <thuth@redhat.com>,
	qemu-devel@nongnu.org, Kevin Wolf <kwolf@redhat.com>,
	Hanna Reitz <hreitz@redhat.com>,
	qemu-block@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH 0/5] iotests: make meson aware of individual I/O tests
Date: Fri, 3 Mar 2023 13:06:16 +0000	[thread overview]
Message-ID: <ZAHwyFhy+shNjfav@redhat.com> (raw)
In-Reply-To: <ZAHP1I82r/cbuUo4@redhat.com>

On Fri, Mar 03, 2023 at 10:45:40AM +0000, Daniel P. Berrangé wrote:
> On Fri, Mar 03, 2023 at 09:30:39AM +0100, Thomas Huth wrote:
> > On 02/03/2023 19.46, Daniel P. Berrangé wrote:
> > 3) When I tried this last year, I had a weird problem that
> >    the terminal sometimes gets messed up ... I wasn't able
> >    to track it down back then - could you check by running
> >    "make check-block" many times (>10 times) to see whether
> >    it happens with your series or not?
> 
> I've just seen this - echo got disabled on my terminal.

The problem is that testrunner.py script doing

# We want to save current tty settings during test run,
# since an aborting qemu call may leave things screwed up.
@contextmanager
def savetty() -> Iterator[None]:
    isterm = sys.stdin.isatty()
    if isterm:
        fd = sys.stdin.fileno()
        attr = termios.tcgetattr(fd)

    try:
        yield
    finally:
        if isterm:
            termios.tcsetattr(fd, termios.TCSADRAIN, attr)


When invoking 'check' this wraps around execution of the entire
'check' script. IOW it saves/restores the terminal once.

When we invoke 'check' in parallel it will save/restore the same
terminal for each invokation.

If the 'termios.tcgetattr' call runs at the same time as there is
a QEMU running which has put the terminal in raw mode, then when
'check' exits it'll "restore" the terminal to raw mode.

IOW, this savetty() logic is fundamentally unsafe when invoking
'check' in parallel.

AFAICT, the more reliable thing todo would be to spawn a new PTY
when invoking each test, so there's no cleanup required. If QEMU
aborts leaving the TTY messed up it dosn't matter was it was a
temporary throwaway PTY.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  reply	other threads:[~2023-03-03 13:06 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-02 18:46 [PATCH 0/5] iotests: make meson aware of individual I/O tests Daniel P. Berrangé
2023-03-02 18:46 ` [PATCH 1/5] iotests: explicitly pass source/build dir to 'check' command Daniel P. Berrangé
2023-03-03 12:55   ` Alex Bennée
2023-03-03 13:01     ` Daniel P. Berrangé
2023-03-02 18:46 ` [PATCH 2/5] iotests: allow test discovery before building Daniel P. Berrangé
2023-03-03  8:14   ` Thomas Huth
2023-03-03 12:56   ` Alex Bennée
2023-03-02 18:46 ` [PATCH 3/5] iotests: strip subdir path when listing tests Daniel P. Berrangé
2023-03-03 12:58   ` Alex Bennée
2023-03-02 18:46 ` [PATCH 4/5] iotests: print TAP protocol version when reporting tests Daniel P. Berrangé
2023-03-03  8:17   ` Thomas Huth
2023-03-03 12:58   ` Alex Bennée
2023-03-02 18:46 ` [PATCH 5/5] iotests: register each I/O test separately with meson Daniel P. Berrangé
2023-03-03  9:34   ` Thomas Huth
2023-03-03 10:21     ` Daniel P. Berrangé
2023-03-02 18:54 ` [PATCH 0/5] iotests: make meson aware of individual I/O tests Daniel P. Berrangé
2023-03-03  8:30 ` Thomas Huth
2023-03-03  8:53   ` Daniel P. Berrangé
2023-03-03  9:39     ` Daniel P. Berrangé
2023-03-03 10:27     ` Thomas Huth
2023-03-03 10:45   ` Daniel P. Berrangé
2023-03-03 13:06     ` Daniel P. Berrangé [this message]
2023-03-03 15:49       ` Thomas Huth
2023-03-03 15:52         ` Daniel P. Berrangé

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=ZAHwyFhy+shNjfav@redhat.com \
    --to=berrange@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /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).