Peter Xu <[email protected]> writes: > On Mon, Feb 02, 2026 at 07:40:52PM -0300, Fabiano Rosas wrote: >> Hi, just a few more patches until options.c is fully using QAPI_CLONE >> instead of open-coding options handling, we're almost there. > > While I'm reading your series, I found one thing that migration might be > racy against for a while, and this series should enlarge that window of > this issue: I don't think it's safe to free elements in MigrationParameters > in the main thread, even if we hold BQL. Because migration thread can > access parameters anytime without BQL... logically it can read any > parameter, then if it's not a scalar we need some luck not crashing. >
Good point, I had thought of it and convinced myself it was ok somehow. I'll take another look. > IMHO we may need to move faster on the "whitelist that can be dynamically > set during migration" plan on migration parameters and capabilities. > Or we make set-parameters always work on top of temporary MigrationParameter objects and switch the pointer atomically. Might be a good idea regardless. > I don't think any cap (after converted to parameters) should be on this > whitelist... for parameters, I'm trying to be conservative but I believe > the list should look like this: > > CPU throttle knobs: > > throttle-trigger-threshold > cpu-throttle-increment > cpu-throttle-tailslow > max-cpu-throttle > x-vcpu-dirty-limit-period > vcpu-dirty-limit > > Bandwith knobs: > > max-bandwidth > avail-switchover-bandwidth > downtime-limit > max-postcopy-bandwidth > > COLO knob: > > x-checkpoint-delay > > That really should be all parameters we allow to change.. Luckily all of > them are scalars, so it's fine to keep the current migrate_params_free() > and logic to "free then update". > > I wonder if we should even consider having this whitelist alongside with > your this series if we want to be extra safe, but I'll let you decide.. I'm > also OK we do it later, but maybe we don't want it to be too late either. Yeah.. I'll see what I can do. Thanks for the reviews this far!
