Hi! Thanks for your notes. I'll try to send updated patches by the end of
the day.
On Fri, Oct 7, 2022 at 6:32 PM Peter Maydell <peter.mayd...@linaro.org>
wrote:
> 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.
--
Best Regards,
*Nikita Ivanov* | C developer
