On Donnerstag, 4. August 2022 09:25:17 CEST Nikita Ivanov wrote:
> From 0ceb04ada1ed5a863914f4449469d7572d3443ed Mon Sep 17 00:00:00 2001
> From: Nikita Ivanov <niva...@cloudlinux.com>
> Date: Wed, 3 Aug 2022 12:54:00 +0300
> Subject: [PATCH] error handling: Use TFR() macro where applicable
>
> There is a defined TFR() macro in qemu/osdep.h which
> handles the same while loop.
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/415
>
> Signed-off-by: Nikita Ivanov <niva...@cloudlinux.com>
> ---
> hw/9pfs/9p-local.c | 8 ++------
> net/l2tpv3.c | 15 +++------------
> net/tap.c | 8 ++------
> qga/commands-posix.c | 4 +---
> util/main-loop.c | 4 +---
> util/osdep.c | 4 +---
> 6 files changed, 10 insertions(+), 33 deletions(-)
I was thinking the same as Marc-André before:
commit 1dacd88ddcf33eb6ed044c4080e3ef5e3de4b6b6
Author: Marc-André Lureau <marcandre.lur...@redhat.com>
Date: Wed Mar 23 19:57:27 2022 +0400
include: move TFR to osdep.h
The macro requires EINTR, which has its header included in osdep.h.
(Not sure what TFR stands for, perhaps "Test For Retry". Rename it ?)
Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
Message-Id: <20220323155743.1585078-17-marcandre.lur...@redhat.com>
Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
Wouldn't it make sense to first rename TFR() to something like
RETRY_ON_EINTR() and then doing this consolidation here on top?
Best regards,
Christian Schoenebeck
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index d42ce6d8b8..c90ab947ba 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -470,9 +470,7 @@ static ssize_t local_readlink(FsContext *fs_ctx,
> V9fsPath *fs_path,
> if (fd == -1) {
> return -1;
> }
> - do {
> - tsize = read(fd, (void *)buf, bufsz);
> - } while (tsize == -1 && errno == EINTR);
> + TFR(tsize = read(fd, (void *)buf, bufsz));
> close_preserve_errno(fd);
> } else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) ||
> (fs_ctx->export_flags & V9FS_SM_NONE)) {
> @@ -908,9 +906,7 @@ static int local_symlink(FsContext *fs_ctx, const char
> *oldpath,
> }
> /* Write the oldpath (target) to the file. */
> oldpath_size = strlen(oldpath);
> - do {
> - write_size = write(fd, (void *)oldpath, oldpath_size);
> - } while (write_size == -1 && errno == EINTR);
> + TFR(write_size = write(fd, (void *)oldpath, oldpath_size));
> close_preserve_errno(fd);
>
> if (write_size != oldpath_size) {
> diff --git a/net/l2tpv3.c b/net/l2tpv3.c
> index af373e5c30..adfdbdb84c 100644
> --- a/net/l2tpv3.c
> +++ b/net/l2tpv3.c
> @@ -240,9 +240,7 @@ static ssize_t
> net_l2tpv3_receive_dgram_iov(NetClientState *nc,
> message.msg_control = NULL;
> message.msg_controllen = 0;
> message.msg_flags = 0;
> - do {
> - ret = sendmsg(s->fd, &message, 0);
> - } while ((ret == -1) && (errno == EINTR));
> + TFR(ret = sendmsg(s->fd, &message, 0));
> if (ret > 0) {
> ret -= s->offset;
> } else if (ret == 0) {
> @@ -285,9 +283,7 @@ static ssize_t net_l2tpv3_receive_dgram(NetClientState
> *nc,
> message.msg_control = NULL;
> message.msg_controllen = 0;
> message.msg_flags = 0;
> - do {
> - ret = sendmsg(s->fd, &message, 0);
> - } while ((ret == -1) && (errno == EINTR));
> + TFR(ret = sendmsg(s->fd, &message, 0));
> if (ret > 0) {
> ret -= s->offset;
> } else if (ret == 0) {
> @@ -434,12 +430,7 @@ static void net_l2tpv3_send(void *opaque)
>
> msgvec = s->msgvec + s->queue_head;
> if (target_count > 0) {
> - do {
> - count = recvmmsg(
> - s->fd,
> - msgvec,
> - target_count, MSG_DONTWAIT, NULL);
> - } while ((count == -1) && (errno == EINTR));
> + TFR(count = recvmmsg(s->fd, msgvec, target_count, MSG_DONTWAIT,
> NULL));
> if (count < 0) {
> /* Recv error - we still need to flush packets here,
> * (re)set queue head to current position
> diff --git a/net/tap.c b/net/tap.c
> index b3ddfd4a74..b047eca8b5 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -102,9 +102,7 @@ static ssize_t tap_write_packet(TAPState *s, const
> struct iovec *iov, int iovcnt
> {
> ssize_t len;
>
> - do {
> - len = writev(s->fd, iov, iovcnt);
> - } while (len == -1 && errno == EINTR);
> + TFR(len = writev(s->fd, iov, iovcnt));
>
> if (len == -1 && errno == EAGAIN) {
> tap_write_poll(s, true);
> @@ -577,9 +575,7 @@ static int net_bridge_run_helper(const char *helper,
> const char *bridge,
>
> close(sv[1]);
>
> - do {
> - fd = recv_fd(sv[0]);
> - } while (fd == -1 && errno == EINTR);
> + TFR(fd = recv_fd(sv[0]));
> saved_errno = errno;
>
> close(sv[0]);
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 954efed01b..90f83aa9b6 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -68,9 +68,7 @@ static void ga_wait_child(pid_t pid, int *status, Error
> **errp)
>
> *status = 0;
>
> - do {
> - rpid = waitpid(pid, status, 0);
> - } while (rpid == -1 && errno == EINTR);
> + TFR(rpid = waitpid(pid, status, 0));
>
> if (rpid == -1) {
> error_setg_errno(errp, errno, "failed to wait for child (pid: %d)",
> diff --git a/util/main-loop.c b/util/main-loop.c
> index f00a25451b..58e14db2d4 100644
> --- a/util/main-loop.c
> +++ b/util/main-loop.c
> @@ -64,9 +64,7 @@ static void sigfd_handler(void *opaque)
> ssize_t len;
>
> while (1) {
> - do {
> - len = read(fd, &info, sizeof(info));
> - } while (len == -1 && errno == EINTR);
> + TFR(len = read(fd, &info, sizeof(info)));
>
> if (len == -1 && errno == EAGAIN) {
> break;
> diff --git a/util/osdep.c b/util/osdep.c
> index 60fcbbaebe..d35c473ac7 100644
> --- a/util/osdep.c
> +++ b/util/osdep.c
> @@ -244,9 +244,7 @@ static int qemu_lock_fcntl(int fd, int64_t start,
> int64_t len, int fl_type)
> .l_type = fl_type,
> };
> qemu_probe_lock_ops();
> - do {
> - ret = fcntl(fd, fcntl_op_setlk, &fl);
> - } while (ret == -1 && errno == EINTR);
> + TFR(ret = fcntl(fd, fcntl_op_setlk, &fl));
> return ret == -1 ? -errno : 0;
> }
