public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [PATCH] qrap: Don't rely on errno after perror(), and reset it before usage
@ 2022-07-06  6:13 Stefano Brivio
  0 siblings, 0 replies; only message in thread
From: Stefano Brivio @ 2022-07-06  6:13 UTC (permalink / raw)
  To: passt-dev

[-- Attachment #1: Type: text/plain, Size: 2463 bytes --]

In commit fca5e11773d0 ("qrap: Add probe retry on connection reset
from passt for KubeVirt integration") I just used errno to check if
the connection was reset on recv(), but perror() might set it to
EINVAL if e.g. an underlying logging mechanism fails, so we won't
actually catch the connection reset.

And in case recv() returns 0, errno won't be set, but we're still
using it without resetting it first, which leads to unpredictable
results in that case.

Reset errno before probing with connect(), send() and recv(), and
save it for later checks before calling perror().

Fixes: fca5e11773d0 ("qrap: Add probe retry on connection reset from passt for KubeVirt integration")
Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
---
 qrap.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/qrap.c b/qrap.c
index 4173281..d73cf6c 100644
--- a/qrap.c
+++ b/qrap.c
@@ -112,8 +112,8 @@ void usage(const char *name)
  */
 int main(int argc, char **argv)
 {
+	int i, s, qemu_argc = 0, addr_map = 0, has_dev = 0, retry_on_reset, err;
 	struct timeval tv = { .tv_sec = 0, .tv_usec = (long)(500 * 1000) };
-	int i, s, qemu_argc = 0, addr_map = 0, has_dev = 0, retry_on_reset;
 	char *qemu_argv[ARG_MAX], dev_str[ARG_MAX];
 	struct sockaddr_un addr = {
 		.sun_family = AF_UNIX,
@@ -249,14 +249,21 @@ retry:
 			perror("setsockopt SO_SNDTIMEO");
 
 		snprintf(addr.sun_path, UNIX_PATH_MAX, UNIX_SOCK_PATH, i);
-		if (connect(s, (const struct sockaddr *)&addr, sizeof(addr)))
+
+		errno = 0;
+
+		if (connect(s, (const struct sockaddr *)&addr, sizeof(addr))) {
+			err = errno;
 			perror("connect");
-		else if (send(s, &probe, sizeof(probe), 0) != sizeof(probe))
+		} else if (send(s, &probe, sizeof(probe), 0) != sizeof(probe)) {
+			err = errno;
 			perror("send");
-		else if (recv(s, &probe_r, 1, MSG_PEEK) <= 0)
+		} else if (recv(s, &probe_r, 1, MSG_PEEK) <= 0) {
+			err = errno;
 			perror("recv");
-		else
+		} else {
 			break;
+		}
 
 		/* FIXME: in a KubeVirt environment, libvirtd invokes qrap three
 		 * times in a strict sequence when a virtual machine needs to
@@ -280,7 +287,7 @@ retry:
 		 * this FIXME will probably remain until the tool itself is
 		 * obsoleted.
 		 */
-		if (retry_on_reset && errno == ECONNRESET) {
+		if (retry_on_reset && err == ECONNRESET) {
 			retry_on_reset--;
 			usleep(50 * 1000);
 			goto retry;
-- 
@@ -112,8 +112,8 @@ void usage(const char *name)
  */
 int main(int argc, char **argv)
 {
+	int i, s, qemu_argc = 0, addr_map = 0, has_dev = 0, retry_on_reset, err;
 	struct timeval tv = { .tv_sec = 0, .tv_usec = (long)(500 * 1000) };
-	int i, s, qemu_argc = 0, addr_map = 0, has_dev = 0, retry_on_reset;
 	char *qemu_argv[ARG_MAX], dev_str[ARG_MAX];
 	struct sockaddr_un addr = {
 		.sun_family = AF_UNIX,
@@ -249,14 +249,21 @@ retry:
 			perror("setsockopt SO_SNDTIMEO");
 
 		snprintf(addr.sun_path, UNIX_PATH_MAX, UNIX_SOCK_PATH, i);
-		if (connect(s, (const struct sockaddr *)&addr, sizeof(addr)))
+
+		errno = 0;
+
+		if (connect(s, (const struct sockaddr *)&addr, sizeof(addr))) {
+			err = errno;
 			perror("connect");
-		else if (send(s, &probe, sizeof(probe), 0) != sizeof(probe))
+		} else if (send(s, &probe, sizeof(probe), 0) != sizeof(probe)) {
+			err = errno;
 			perror("send");
-		else if (recv(s, &probe_r, 1, MSG_PEEK) <= 0)
+		} else if (recv(s, &probe_r, 1, MSG_PEEK) <= 0) {
+			err = errno;
 			perror("recv");
-		else
+		} else {
 			break;
+		}
 
 		/* FIXME: in a KubeVirt environment, libvirtd invokes qrap three
 		 * times in a strict sequence when a virtual machine needs to
@@ -280,7 +287,7 @@ retry:
 		 * this FIXME will probably remain until the tool itself is
 		 * obsoleted.
 		 */
-		if (retry_on_reset && errno == ECONNRESET) {
+		if (retry_on_reset && err == ECONNRESET) {
 			retry_on_reset--;
 			usleep(50 * 1000);
 			goto retry;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2022-07-06  6:13 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-06  6:13 [PATCH] qrap: Don't rely on errno after perror(), and reset it before usage Stefano Brivio

Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

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 IMAP folder(s).