Re: [PATCH v6 01/34] qom: replace 'can_be_deleted' with 'prepare_delete'

[Markus Armbruster <armbru@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> While most objects can perform all their cleanup in the finalizer
> method, there can be interactions with other resources / subsystems
> / threads which require that some cleanup be performed on an user
> creatable object before unparenting it and entering finalization.
>
> The current 'can_be_deleted' method runs in the deletion path and
> is intended to be used to block deletion. While it could be used
> to perform cleanup tasks, its name suggests it should be free of
> side-effects.
>
> Generalize this by renaming it to 'prepare_delete', explicitly
> allowing for cleanup to be provided. Existing users of 'can_be_deleted'
> are re-written, which provides them with more detailed/tailored error
> messages.

Old message:

        error_setg(errp, "object '%s' is in use, can not be deleted", id);

New messages:

        error_setg(errp, "Cryptodev backend '%s' is still in use",
        error_setg(errp, "Host memory backend '%s' is still mapped",
        error_setg(errp, "IOMMUFD backend '%s' still has %d users",
        error_setg(errp, "Throttle group '%s' still has multiple references",
    error_setg(errp, "Deleting CAN bus device '%s' is not supported",
    error_setg(errp, "Deleting main loop '%s' is not supported",

All now mention what kind of object it is.  Users could probably figure
this out from context easily.  Welcome small improvement.

The two unconditional ones become more accurate: they now report "not
supported" instead of "in use".

The other four lose "can not be deleted".  Users can probably figure
this out from context.  Still, I lean towards error messages like "Can't
DO-SOMETHING because REASON" until that becomes overly verbose.  What do
you think?

> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

[...]

> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index 7080f85f95..ac992aa786 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -32,16 +32,15 @@ bool user_creatable_complete(UserCreatable *uc, Error **errp)
>      return !*errp;
>  }
>  
> -bool user_creatable_can_be_deleted(UserCreatable *uc)
> +bool user_creatable_prepare_delete(UserCreatable *uc, Error **errp)
>  {
> -
>      UserCreatableClass *ucc = USER_CREATABLE_GET_CLASS(uc);
> +    ERRP_GUARD();

No longer necessary.

>  
> -    if (ucc->can_be_deleted) {
> -        return ucc->can_be_deleted(uc);
> -    } else {
> -        return true;
> +    if (ucc->prepare_delete) {
> +        return ucc->prepare_delete(uc, errp);
>      }
> +    return true;
>  }
>  
>  void user_creatable_add_qapi(ObjectOptions *options, Error **errp)
> @@ -253,8 +252,7 @@ bool user_creatable_del(const char *id, Error **errp)
>          return false;
>      }
>  
> -    if (!user_creatable_can_be_deleted(USER_CREATABLE(obj))) {
> -        error_setg(errp, "object '%s' is in use, can not be deleted", id);
> +    if (!user_creatable_prepare_delete(USER_CREATABLE(obj), errp)) {
>          return false;
>      }
>  

[...]