qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Ed Swierk <eswierk@skyportsystems.com>
Cc: qemu-devel@nongnu.org, libvir-list@redhat.com,
	pbonzini@redhat.com, marcandre.lureau@redhat.com,
	Eric Blake <eblake@redhat.com>
Subject: Re: [Qemu-devel] [libvirt] char: Logging serial pty output when disconnected
Date: Tue, 31 Jan 2017 09:39:27 +0000	[thread overview]
Message-ID: <20170131093927.GA20303@redhat.com> (raw)
In-Reply-To: <CAO_EM_no14RDCd42hJy1xbo=mYsdo8k7oAESVx2KpF277U45VQ@mail.gmail.com>

On Mon, Jan 30, 2017 at 08:58:25PM -0800, Ed Swierk wrote:
> On Fri, Jan 27, 2017 at 1:40 AM, Daniel P. Berrange <berrange@redhat.com> wrote:
> > On Thu, Jan 26, 2017 at 05:07:16PM -0800, Ed Swierk wrote:
> >> Currently qemu_chr_fe_write() calls qemu_chr_fe_write_log() only for
> >> data consumed by the backend chr_write function. With the pty backend,
> >> pty_chr_write() returns 0 indicating that the data was not consumed
> >> when the pty is disconnected. Simply changing it to return len instead
> >> of 0 tricks the caller into logging the data even when the pty is
> >> disconnected. I don't know what problems this might cause, but one
> >> data point is that tcp_chr_write() already happens to work this way.
> >>
> >> Alternatively, qemu_chr_fe_write() could be modified to log everything
> >> passed to it, regardless of how much data chr_write claims to have
> >> consumed. The trouble is that the serial device retries writing
> >> unconsumed data, so when the pty is disconnected you'd see every
> >> character duplicated 4 times in the log file.
> >>
> >> Any opinions on either approach, or other suggestions? If there are no
> >> objections to the first one, I'll prepare a patch.
> >
> > If the pty backend intends to just drop data into a blackhole when
> > no client is connected, then its chr_write() impl should return
> > the length of the data discarded, not zero.
> 
> That's exactly the question: when no client is connected, should the
> pty backend just drop the data into a black hole, returning the length
> of the data discarded? Or should it return 0, letting the frontend
> device decide what to do with it?

It should return len of data discarded.

> 
> I can't discern a consistent pattern across all the char backends. The
> closest analog is the tcp backend, which does discard the data and
> return len. In contrast, several backends call
> io_channel_send{,_full}(), which returns -1 if the write would block
> or fails for any other reason.
> 
> It's not clear there's much the frontend can do to recover from an
> error, but there's no consistent pattern across serial devices either.
> Most just ignore the return value. But the 16550A serial device
> retries 4 times after an error. Changing the pty backend to discard
> the data on the first attempt would bypass this retry mechanism. Is
> that a problem?

I don't think so - retrying in this way is pointless IMHO - it is just
going to get the same result on every retry on 99% of occassions.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

  reply	other threads:[~2017-01-31  9:39 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-27  1:07 [Qemu-devel] char: Logging serial pty output when disconnected Ed Swierk
2017-01-27  9:40 ` [Qemu-devel] [libvirt] " Daniel P. Berrange
2017-01-31  4:58   ` Ed Swierk
2017-01-31  9:39     ` Daniel P. Berrange [this message]
2017-01-31 15:34       ` Paolo Bonzini
2017-01-31 22:03         ` Ed Swierk

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=20170131093927.GA20303@redhat.com \
    --to=berrange@redhat.com \
    --cc=eblake@redhat.com \
    --cc=eswierk@skyportsystems.com \
    --cc=libvir-list@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@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).