On Wed, 21 Jan 2026 at 18:12, Fabiano Rosas <[email protected]> wrote:
> In general, I think that whenever we determine what protects a data
> structure from concurrent access we should make it obvious.
>
> So this assert is here to provide _some_ assurance that this routine is
> protected.
* Yes, but it is not obvious that we are checking bql_locked() to
protect from concurrent access to the global MigrationState object.
Secondly for concurrent access, other threads should be running.
===
Thread 1 "qemu-system-x86" hit Breakpoint 1, migrate_params_apply
(params=0x7ffcb6627840) at ../migration/options.c:1392
1392 {
(gdb) n
1393 MigrationState *s = migrate_get_current();
(gdb) p bql_locked()
$3 = true
(gdb) p migration_is_running()
$4 = false
(gdb)
Thread 1 "qemu-system-x86" hit Breakpoint 1, migrate_params_apply
(params=0x7ffcb6627840) at ../migration/options.c:1392
1392 {
(gdb) n
1393 MigrationState *s = migrate_get_current();
(gdb) p bql_locked()
$5 = true
(gdb) p migration_is_running()
$6 = false
(gdb)
===
* migrate_params_apply() is called twice:
1. At the beginning when libvirtd(8) initiates migration
2. To reset migration options if we kill migration on the
destination side by killing VM or connection.
Both times it is called from the main Thread-1, BQL is locked and
other migration threads are not running.
* If we are trying to futureproof this function so that no one should
be able to call migrate_params_apply() from other threads without bql
- that seems like a long shot. Because it is reachable only from
qmp_migrate_set_parameters(), which is invoked by an external QMP
user. Ie. someone would have to explicitly send
qmp_migrate_set_parameters() command while migration is running, but
even then it'll still be invoked by the main thread-1, with
bql_locked=true. no?
* I'm thinking maybe we can skip assert(bql_locked()) and related
header inclusion.
Thank you.
---
- Prasad
