From: Zhu Yangyang <zhuyangyan...@huawei.com>
On Mon, 24 Mar 2025 14:09:13 -0400, Stefan Hajnoczi wrote:
> On Fri, Mar 21, 2025 at 03:09:16PM +0800, zoudongjie wrote:
> > From: Zhu Yangyang <zhuyangyan...@huawei.com>
> >
> > The bdrv_drained_begin() function is a blocking function. In scenarios
> > where network storage
> > is used and network links fail, it may block for a long time.
> > Therefore, we add a timeout parameter to control the duration of the block.
> >
> > Since bdrv_drained_begin() has been widely adopted, both
> > bdrv_drained_begin()
> > and bdrv_drained_begin_timeout() will be retained.
> ...
>
> > +/**
> > + * AIO_WAIT_WHILE_TIMEOUT:
> > + *
> > + * Refer to the implementation of AIO_WAIT_WHILE_INTERNAL,
> > + * the timeout parameter is added.
> > + * @timeout_ns: maximum duration to wait, in nanoseconds, except the value
> > + * is unsigned, 0 means infinite.
> > + */
> > +#define AIO_WAIT_WHILE_TIMEOUT(ctx, cond, timeout_ns) ({ \
> > + int ret_ = 0; \
> > + uint64_t timeout_ = (timeout_ns); \
> > + AioWait *wait_ = &global_aio_wait; \
> > + AioContext *ctx_ = (ctx); \
> > + AioContext *current_ctx_ = NULL; \
> > + QEMUTimer timer_; \
> > + /* Increment wait_->num_waiters before evaluating cond. */ \
> > + qatomic_inc(&wait_->num_waiters); \
> > + /* Paired with smp_mb in aio_wait_kick(). */ \
> > + smp_mb__after_rmw(); \
> > + if (ctx_ && in_aio_context_home_thread(ctx_)) { \
> > + current_ctx_ = ctx_; \
> > + } else { \
> > + assert(qemu_get_current_aio_context() == \
> > + qemu_get_aio_context()); \
> > + current_ctx_ = qemu_get_aio_context(); \
> > + } \
> > + if (timeout_ > 0) { \
> > + timer_init_full(&timer_, ¤t_ctx_->tlg, \
> > + QEMU_CLOCK_REALTIME, \
> > + SCALE_NS, 0, aio_wait_timer_cb, NULL); \
> > + timer_mod_ns(&timer_, \
> > + qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + \
> > + timeout_); \
> > + } \
> > + while ((cond)) { \
> > + aio_poll(current_ctx_, true); \
> > + if (timeout_ > 0 && !timer_pending(&timer_)) { \
> > + ret_ = -ETIMEDOUT; \
> > + break; \
> > + } \
> > + } \
> > + if (timeout_ > 0) { \
> > + timer_del(&timer_); \
> > + } \
> > + qatomic_dec(&wait_->num_waiters); \
> > + ret_; })
> > +
> > #define AIO_WAIT_WHILE(ctx, cond) \
> > AIO_WAIT_WHILE_INTERNAL(ctx, cond)
>
> It would be nice to unify AIO_WAIT_WHILE_INTERNAL() and
> AIO_WAIT_WHILE_TIMEOUT(). I don't see any caller that uses the waited_
> return value from AIO_WAIT_WHILE_INTERNAL(), so I think it's possible to
> replace AIO_WAIT_WHILE_INTERNAL() with AIO_WAIT_WHILE_TIMEOUT(.., ..., 0).
>
Yes, there is no difference between AIO_WAIT_WHILE_INTERNAL() and
AIO_WAIT_WHILE_TIMEOUT() except the return value. It's right to keep the
code uniform. I will modify it in the next version.
