Hi
On Fri, Aug 5, 2022 at 3:11 PM Christian Schoenebeck
<qemu_...@crudebyte.com> wrote:
>
> 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?
Apparently TFR often stands for "Temp Failure Retry" (looking at
github code search)
LOOP_WHILE_EINTR ?
>
> 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;
> > }
>
>
