Hi! Thanks for your notes. I'll try to send updated patches by the end of the day.
I think this patch is doing things in the wrong order. Instead of
converting code to use the old macro that we don't like and then
updating it again in patch 2 to use the new macro, we should
first introduce the new macro, and then after that we can update
code that's currently not using a macro at all to use the new one.
This makes code review easier because we don't have to look at a
change to this code which is then going to be rewritten anyway.
Sounds smooth. I'll refactor patches accordingly.
> if (ret < 0) {
> ret = -errno;
> @@ -1472,8 +1472,8 @@ static ssize_t handle_aiocb_rw_vector(RawPosixAIOData *aiocb)
> {
> ssize_t len;
>
> - TFR(
> - len = (aiocb->aio_type & QEMU_AIO_WRITE) ?
> + len = TEMP_FAILURE_RETRY(
> + (aiocb->aio_type & QEMU_AIO_WRITE) ?
> qemu_pwritev(aiocb->aio_fildes,
> aiocb->io.iov,
> aiocb->io.niov,
I'm not sure why you've put the TEMP_FAILURE_RETRY on the outside here
rather than just on the individual function calls.
The original code contained both branches in one while loop, so I was afraid that
value `aiocb->aio_type & QEMU_AIO_WRITE` would change somehow during the loop.
If you'll say that this is impossible, I'll adjust the code as you propose.
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index b1c161c035..6e244f15fa 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -243,7 +243,13 @@ void QEMU_ERROR("code path is reachable")
> #define ESHUTDOWN 4099
> #endif
>
> -#define TFR(expr) do { if ((expr) != -1) break; } while (errno == EINTR)
> +#define TEMP_FAILURE_RETRY(expr) \
We can't call the macro this, because the glibc system headers already
may define a macro of that name, so the compiler will complain if they're
both defined at the same time, and depending on header ordering it might
not be clear which version you're getting.
Sorry, my fault. I will rename it to "RETRY_ON_EINTR" as it was proposed earlier in this thread.