From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 51786C2A074 for ; Mon, 5 Jan 2026 10:21:28 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1vchhY-0001NO-BW; Mon, 05 Jan 2026 05:20:40 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1vchhU-0001N3-QX for qemu-devel@nongnu.org; Mon, 05 Jan 2026 05:20:36 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1vchhS-00055c-OP for qemu-devel@nongnu.org; Mon, 05 Jan 2026 05:20:36 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1767608432; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=d2+oPgeug8ojuyH1YND3koQilsALoLQNsHCab73EJq0=; b=CnKOhiTP6+xdgBE18cUvfmJkC2dwfWi4nuNZHw2i/6Ic8Bsr6svhzUVCwcLJGFuNhq7hxN CthWrtoTCsrtAu4QWc6KWoKHIwId787H9dPrL6Jc/O11sdUpD2bPJClqIkoAUHQJLOHNOR /dJrlx0/qHDCxMo0wZNkfuUVg888vFc= Received: from mx-prod-mc-08.mail-002.prod.us-west-2.aws.redhat.com (ec2-35-165-154-97.us-west-2.compute.amazonaws.com [35.165.154.97]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-249-dEIyH4GbN1qTlFuwwWvExQ-1; Mon, 05 Jan 2026 05:20:30 -0500 X-MC-Unique: dEIyH4GbN1qTlFuwwWvExQ-1 X-Mimecast-MFC-AGG-ID: dEIyH4GbN1qTlFuwwWvExQ_1767608429 Received: from mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.17]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-08.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 769A61800623; Mon, 5 Jan 2026 10:20:29 +0000 (UTC) Received: from redhat.com (unknown [10.42.28.159]) by mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 79D0419560AB; Mon, 5 Jan 2026 10:20:27 +0000 (UTC) Date: Mon, 5 Jan 2026 10:20:23 +0000 From: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= To: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Cc: luzhipeng , qemu-devel@nongnu.org, Paolo Bonzini Subject: Re: [PATCH] chardev/pty: fix incorrect return value in char_pty_chr_write Message-ID: References: <20251225063412.572-1-luzhipeng@cestc.cn> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/2.2.14 (2025-02-20) X-Scanned-By: MIMEDefang 3.0 on 10.30.177.17 Received-SPF: pass client-ip=170.10.129.124; envelope-from=berrange@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org On Fri, Dec 26, 2025 at 03:55:19PM +0400, Marc-André Lureau wrote: > Hi > > On Thu, Dec 25, 2025 at 10:41 AM luzhipeng wrote: > > > In char_pty_chr_write(), when the PTY is not connected (s->connected == > > false), the function attempts a non-blocking poll to check if the fd is > > writable. However, even if the fd is not writable or if io_channel_send() > > fails the function unconditionally returns 'len', falsely indicating that > > all data was successfully written. > > > > Signed-off-by: luzhipeng > > > > > > > > --- > > chardev/char-pty.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/chardev/char-pty.c b/chardev/char-pty.c > > index 652b0bd9e7..37a20d5f4b 100644 > > --- a/chardev/char-pty.c > > +++ b/chardev/char-pty.c > > @@ -124,11 +124,15 @@ static int char_pty_chr_write(Chardev *chr, const > > uint8_t *buf, int len) > > pfd.revents = 0; > > rc = RETRY_ON_EINTR(g_poll(&pfd, 1, 0)); > > g_assert(rc >= 0); > > + if ((pfd.revents & G_IO_HUP) || !(pfd.revents & G_IO_OUT)) { > > + return 0; > > + } > > if (!(pfd.revents & G_IO_HUP) && (pfd.revents & G_IO_OUT)) { > > return io_channel_send(s->ioc, buf, len); > > } > > > > - return len; > > + int ret = io_channel_send(s->ioc, buf, len); > > + return ret; > > > > > The 'ret' variable is probably unnecessary. > > I suppose this will return 0 in general (unless the fd state has changed > after the poll) > > So this will likely conflict the behaviour change from: I believe behaviour can be worse than the commit below indicates. If we don't consume data sent by the serial port, then IIRC, wit at least some serial device impls we'll get a full buffer and hang the guest. Consider the semantics with a socket based chardev - we'll throw away all data from the serial port when no client is connected. We wanted the same behaviour with the PTY. if nothing is connected to the other end of the PTY, we want to discard all data. Always returning 'len' is achieving that by pretending we wrote the data. > > commit 1c64fdbc8177058802df205f5d7cd65edafa59a8 > Author: Ed Swierk > Date: Tue Jan 31 05:45:29 2017 -0800 > > char: drop data written to a disconnected pty > > When a serial port writes data to a pty that's disconnected, drop the > data and return the length dropped. This avoids triggering pointless > retries in callers like the 16550A serial_xmit(), and causes > qemu_chr_fe_write() to write all data to the log file, rather than > logging only while a pty client like virsh console happens to be > connected. > > Signed-off-by: Ed Swierk > Message-Id: < > 1485870329-79428-1-git-send-email-eswierk@skyportsystems.com> > Signed-off-by: Paolo Bonzini > > diff --git a/chardev/char-pty.c b/chardev/char-pty.c > index 27eb85f505..ecf2c7a5c4 100644 > --- a/chardev/char-pty.c > +++ b/chardev/char-pty.c > @@ -129,7 +129,7 @@ static int char_pty_chr_write(Chardev *chr, const > uint8_t *buf, int len) > /* guest sends data, check for (re-)connect */ > pty_chr_update_read_handler_locked(chr); > if (!s->connected) { > - return 0; > + return len; 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 :|