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
