On Wed, Feb 25, 2026 at 03:04:27PM +0530, Ani Sinha wrote:
>
>
> > On 25 Feb 2026, at 2:37 PM, Daniel P. Berrangé <[email protected]> wrote:
> >
> > On Wed, Feb 25, 2026 at 11:35:15AM +0530, Prasad Pandit wrote:
> >> On Wed, 25 Feb 2026 at 09:23, Ani Sinha <[email protected]> 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.
> >>>
> >>> Suggested-by: Prasad Pandit <[email protected]>
> >>> Signed-off-by: Ani Sinha <[email protected]>
> >>> ---
> >>> migration/migration.c | 4 ++++
> >>> 1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/migration/migration.c b/migration/migration.c
> >>> index a5b0465ed3..1eb75fb7fb 100644
> >>> --- a/migration/migration.c
> >>> +++ b/migration/migration.c
> >>> @@ -1702,6 +1702,10 @@ static int add_blockers(Error **reasonp, unsigned
> >>> modes, Error **errp)
> >
> > This method has an '**errp' parameter.....
> >
> >>> {
> >>> for (MigMode mode = 0; mode < MIG_MODE__MAX; mode++) {
> >>> if (modes & BIT(mode)) {
> >>> + if (g_slist_index(migration_blockers[mode],
> >>> + *reasonp) >= 0) {
> >>> + return -EEXIST;
> >
> > .... so using -errno for return values is not appropriate - it must
> > set 'errp' and return -1.
>
> I see functions migrate_add_blocker_modes() or migrate_add_blocker_internal()
> that do not do this. This is why I thought it was ok to just return -errno
> here.
Yes they all set 'errp', but indirectly:
if (is_only_migratable(reasonp, modes, errp)) {
return -EACCES;
} else if (is_busy(reasonp, errp)) {
return -EBUSY;
}
In this case 'is_busy' and 'is_only_migratable' will be setting 'errp'.
With regards,
Daniel
--
|: https://berrange.com ~~ https://hachyderm.io/@berrange :|
|: https://libvirt.org ~~ https://entangle-photo.org :|
|: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
