From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 1D6B05A0319 for ; Fri, 26 Jul 2024 14:37:24 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1721997440; bh=kiIhb62bjyb3L/b3COQJcNl5Rueuxy+RAj6mNKUMa0A=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=A4PLiCO/Krgf+bS939A3MjB79FT1MKeZ834boEkmPzpP3zTLfaTOfTxNoAZXAx4gb OyEjTMtx/tRLHCTwU/m9keUrB+y8Ic5TKc9TNoY++/nKdylbisPPfREOJIFCMq/6Wt OyszPE2hUdYTBsyXWh5807kBvdyLTjswPw0SM4Zt/6Rqq43KZP97XilAtCNCWYCzSE uCb8WwSrPYgzux9SK0Ozho0DeN+wn1f5AYvEBMjGctAh8Y9ma5WnWgM9HKEvn0hw/Z dAKfQWlyR5I/gFtfGDoASvRSltt6aVXJHT/WrUD4dENQaGeXDAH20vshVYuwrTXg3L it8dZ16uhXZmA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4WVnMN0Dhpz4x7G; Fri, 26 Jul 2024 22:37:20 +1000 (AEST) Date: Fri, 26 Jul 2024 22:02:30 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 1/5] tap: Better report errors receiving from QEMU socket Message-ID: References: <20240726072031.3941305-1-david@gibson.dropbear.id.au> <20240726072031.3941305-2-david@gibson.dropbear.id.au> <20240726132508.6a4d1147@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="n2AWZ/JeS49zbNPw" Content-Disposition: inline In-Reply-To: <20240726132508.6a4d1147@elisabeth> Message-ID-Hash: HIDI5VYAHUVSVQUXYIL7OLH3IGPMXQSA X-Message-ID-Hash: HIDI5VYAHUVSVQUXYIL7OLH3IGPMXQSA X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --n2AWZ/JeS49zbNPw Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jul 26, 2024 at 01:25:08PM +0200, Stefano Brivio wrote: > On Fri, 26 Jul 2024 17:20:27 +1000 > David Gibson wrote: >=20 > > If we get an error on recv() from the QEMU socket, we currently don't > > print any kind of error. Although this can happen in a non-fatal situa= tion > > such as a guest restarting, it's unusual enough that we realy should re= port > > something for debugability. > >=20 > > Add an error message in this case. Also always report when the qemu > > connection closes for any reason, not just when it will cause us to exit > > (--one-off). > >=20 > > Signed-off-by: David Gibson > > --- > > tap.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > >=20 > > diff --git a/tap.c b/tap.c > > index 44bd4445..a2ae7c2a 100644 > > --- a/tap.c > > +++ b/tap.c > > @@ -969,10 +969,10 @@ void tap_add_packet(struct ctx *c, ssize_t l2len,= char *p) > > */ > > static void tap_sock_reset(struct ctx *c) > > { > > - if (c->one_off) { > > - info("Client closed connection, exiting"); > > + info("Client connection closed%s", c->one_off ? ", exiting" : ""); > > + > > + if (c->one_off) > > exit(EXIT_SUCCESS); > > - } > > =20 > > /* Close the connected socket, wait for a new connection */ > > epoll_ctl(c->epollfd, EPOLL_CTL_DEL, c->fd_tap, NULL); > > @@ -1005,8 +1005,10 @@ redo: > > =20 > > n =3D recv(c->fd_tap, p, TAP_BUF_FILL, MSG_DONTWAIT); > > if (n < 0) { > > - if (errno !=3D EINTR && errno !=3D EAGAIN && errno !=3D EWOULDBLOCK) > > + if (errno !=3D EINTR && errno !=3D EAGAIN && errno !=3D EWOULDBLOCK)= { > > + err_perror("Error receiving from QEMU socket"); >=20 > Nit I'm fixing up on merge: it's not necessarily QEMU, because libkrun > and perhaps others also use this path (in fact, the whole problem was > reported as part of the libkrun integration). Fair point. I'm not sure what term to use to describe specifically a socket using this qemu-originated protocol, though. > Maybe it's obvious to users anyway, but this might seriously confuse > somebody who's using e.g. krun on Asahi Linux (is QEMU running, one > might wonder): > https://github.com/slp/krun#motivation > https://github.com/slp/krun/blob/main/crates/krun/src/net.rs >=20 > On top of that, now that we have an error message, I guess it would be > nice to state we're resetting the connection, because it's not really > obvious: the subsequent message from tap_sock_reset() makes it look like > the client decided to close the connection on its own. That's why I changed the message in tap_sock_reset() from "client closed connection" to "client connection closed". >=20 > So I'm changing this to: >=20 > err("Error receiving from guest, resetting connection"); >=20 > ...if you see an issue with it, I'll send another patch. Ok. --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --n2AWZ/JeS49zbNPw Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmajkDMACgkQzQJF27ox 2Gfh0w/+MBC9BZLdCjRcHrR84JM1beH+Lu3epZ2W+9eU/4eueD9mos0C6dQEVQL1 QZxjwJYBDRjqsYib35CreFUrHrdRC+T/EUktG5XAYUaAGiu33L+B4Rk1cPGftvO2 KpHSYUZMp2c6LrB0VnLtF2FdVM3HDr180mZMEoIfhjn+G6YiRwASFlReE+BLR5kz AODzDWnGFHaNFcpR+W3bPhlO+FSXfeaMZIQEpOqM5CI4eMZpGov42JckEJ1vTZ2b Kq/gEIjQ9tPPd3+4zUji+I+5ovrGvuxqoYzMd9XVrBu0leKL897NGcf6CttEtOMn jgDASoc32PRlauNRAe+32YfH+JexvzmcikgLCUemiOST4H0UXUDqtaJRJwxNGgJ+ J9FIS0OEwUWZX0pMQtMeU43+wiBkkmj2/AHfPySgIrWMAggETEBZ2RiQK35gHQCp l+kiLM9JS4+mA5kwCJ8z6WHh66VULPXdqyJzGmLvrd54d3VaJ7v06Q9IQIgthijg hMVBPajGz0dyXKrUwg3lw1C/DAk3kmUr5j43+KujAauLG8M3+bMo5eNLlO+VvlRQ tqVEMn5ylcbeUE1rw2WhtG/ObpwecAU/temsxWKEMzQaSdokq+RjJsod9e6tQe5I DczsYVa6BqRvHOw1lRN6uFKjrNIKfJ9NOebk3QlXUV22dILttuw= =Zm9U -----END PGP SIGNATURE----- --n2AWZ/JeS49zbNPw--