Hello all,
On Thu, 26 Feb 2026 at 21:46, Ani Sinha <[email protected]> wrote:
> >>> On Wed, Feb 25, 2026 at 09:19:40AM +0530, Ani Sinha wrote:
> >>>> Currently the code that adds a migration blocker does not check if the
> >>>> same
> >>>> blocker already exists. Return an EEXIST error code if there is an
> >>>> attempt to
> >>>> add the same migration blocker again. This way the same migration
> >>>> blocker will
> >>>> not get added twice.
> >>>
> >>> Could you help explain why it will inject two identical errors in the
> >>> first
> >>> place, and why the caller cannot make sure it won't be injected twice?
> >>
> >> Likely due to a bug in the code. For example if the init function that
> >> adds the blocker is called again and the caller does not handle the
> >> second init call properly. This came up as a part of the coco reset work
> >> where migration blockers are added in init methods. They need not be
> >> added again when init methods are again called during the reset
> >> process. The caller can handle it of course but adding a check further
> >> down the call stack makes things more robust.
> >
> > IMHO if we want to make it more robust, we shouldn't return an error
> > because the caller may not always check for errors.
> >
> > Would assertion suites more here? Because migration blockers are not
> > something the user can manipulate, so it's a solid bug to fix when
> > triggered.
>
> If Prasad agrees, I will send something.
* The majority of the places I see constructs like:
===
if (migrate_add_blocker(&g->migration_blocker, errp) < 0) {
OR
ret = migrate_add_blocker(&hv_no_nonarch_cs_mig_blocker, &local_err);
if (ret < 0) {
error_report_err(local_err);
===
* So setting **errp and returning a negative value is consistent with
that. Aborting (assert(3)) for trying to add a duplicate blocker
object seems like a harsh punishment, considering that'll happen at
run time and the user won't be able to do much then.
Thank you.
---
- Prasad
