Re: [PATCH RFC 5/9] migration: Thread-ify precopy vmstate load process

[Peter Xu <peterx@redhat.com>]

On Fri, Aug 29, 2025 at 11:29:59AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > For that, qemu_loadvm_state() and qemu_loadvm_state_main() functions need
> > to now take a "bql_held" parameter saying whether bql is held.  We could
> > use things like BQL_LOCK_GUARD(), but this patch goes with explicit
> > lockings rather than relying on bql_locked TLS variable.  In case of
> > migration, we always know whether BQL is held in different context as long
> > as we can still pass that information downwards.
> 
> Agree, but I think it's better to make new macros following same pattern, i.e.
> 
> WITH_BQL_HELD(bql_held) {
>     action();
> }
> 
> instead of
> 
> WITH_BQL_HELD(bql_held, actions());
> 
> ..
> 
> Or I'm missing something and we already have a precedent of the latter
> notation?

Nop.. it's just that when initially working on that I didn't try as hard to
achieve such pattern.  Here we need to recover the BQL status after the
block, so I didn't immediately see how autoptr would work there.

But I tried slightly harder, I think below should achieve the same pattern
but based on some for() magic.

Thanks for raising this, early comments still be welcomed or I'll go with
that.

===8<===

static inline void
with_bql_held_lock(bool bql_held, const char *file, int line)
{
    assert(bql_held == bql_locked());
    if (!bql_held) {
        bql_lock_impl(file, line);
    }
}

static inline void
with_bql_held_unlock(bool bql_held)
{
    assert(bql_locked());
    if (!bql_held) {
        bql_unlock();
    }
}

/**
 * WITH_BQL_HELD(): Run a block of code, making sure BQL is held
 * @bql_held: Whether BQL is already held
 *
 * Example use case:
 *
 * WITH_BQL_HELD(bql_held) {
 *     // BQL is guaranteed to be held within this block,
 *     // if it wasn't held, will be released when the block finishes.
 * }
 */
#define  WITH_BQL_HELD(bql_held)                                \
    for (bool _bql_once = \
             (with_bql_held_lock(bql_held, __FILE__, __LINE__), true);  \
         _bql_once;                                                     \
         _bql_once = (with_bql_held_unlock(bql_held), false))           \

static inline void
with_bql_released_unlock(bool bql_held)
{
    assert(bql_held == bql_locked());
    if (bql_held) {
        bql_unlock();
    }
}

static inline void
with_bql_released_lock(bool bql_held, const char *file, int line)
{
    assert(!bql_locked());
    if (bql_held) {
        bql_lock_impl(file, line);
    }
}

/**
 * WITH_BQL_RELEASED(): Run a task, making sure BQL is released
 * @bql_held: Whether BQL is already held
 *
 * Example use case:
 *
 * WITH_BQL_RELEASED(bql_held) {
 *     // BQL is guaranteed to be released within this block,
 *     // if it was held, will be re-taken when the block finishes.
 * }
 */
#define  WITH_BQL_RELEASED(bql_held)                                    \
    for (bool _bql_once = (with_bql_released_unlock(bql_held), true);   \
         _bql_once;                                                     \
         _bql_once =                                                    \
             (with_bql_released_lock(bql_held, __FILE__, __LINE__), false)) \

-- 
Peter Xu