On Tue, Feb 07, 2023 at 11:03:13AM -0800, Steve Sistare wrote: > Modify migrate_add_blocker and migrate_del_blocker to take an Error ** > reason. This allows migration to own the Error object, so that if > an error occurs, migration code can free the Error and clear the client > handle, simplifying client code. This is a pre-requisite for a future > patch that will allow one Error blocker to be registered for multiple > migration modes. > > No functional change. > > Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
Acked-by: Peter Xu <pet...@redhat.com> One trivial comment below. [...] > diff --git a/include/migration/blocker.h b/include/migration/blocker.h > index 9cebe2b..7c8d326 100644 > --- a/include/migration/blocker.h > +++ b/include/migration/blocker.h > @@ -17,19 +17,22 @@ > /** > * @migrate_add_blocker - prevent migration from proceeding > * > - * @reason - an error to be returned whenever migration is attempted > + * @reasonp - address of an error to be returned whenever migration is > attempted > * > * @errp - [out] The reason (if any) we cannot block migration right now. > * > * @returns - 0 on success, -EBUSY/-EACCES on failure, with errp set. > + * > + * *@reasonp is freed and set to NULL if failure is returned. > + * On success, the caller no longer owns *@reasonp and must not free it. This statement reads weird. IMHO the caller still owns @reasonp, but if it succeeded it should only free it with a migrate_del_blocker() later. > */ > -int migrate_add_blocker(Error *reason, Error **errp); > +int migrate_add_blocker(Error **reasonp, Error **errp); > > /** > * @migrate_add_blocker_internal - prevent migration from proceeding without > * only-migrate implications > * > - * @reason - an error to be returned whenever migration is attempted > + * @reasonp - address of an error to be returned whenever migration is > attempted > * > * @errp - [out] The reason (if any) we cannot block migration right now. > * > @@ -38,14 +41,19 @@ int migrate_add_blocker(Error *reason, Error **errp); > * Some of the migration blockers can be temporary (e.g., for a few seconds), > * so it shouldn't need to conflict with "-only-migratable". For those > cases, > * we can call this function rather than @migrate_add_blocker(). > + * > + * *@reasonp is freed and set to NULL if failure is returned. > + * On success, the caller no longer owns *@reasonp and must not free it. Same here? > */ -- Peter Xu
