Daniel P. Berrangé <[email protected]> writes: > On Tue, Jun 30, 2026 at 09:34:26AM +0200, Markus Armbruster wrote: >> Daniel P. Berrangé <[email protected]> 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. >> > >> > Reviewed-by: Marc-André Lureau <[email protected]> >> > Tested-by: Peter Krempa <[email protected]> >> > Signed-off-by: Daniel P. Berrangé <[email protected]> >> >> [...] >> >> > diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c >> > index 7080f85f95..6faa0b2fd9 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(); >> > >> > - if (ucc->can_be_deleted) { >> > - return ucc->can_be_deleted(uc); >> > - } else { >> > - return true; >> > + if (ucc->prepare_delete) { >> > + ucc->prepare_delete(uc, errp); >> > } >> > + return !*errp; >> > } >> >> Simpler: >> >> if (ucc->prepare_delete) { >> return ucc->prepare_delete(uc, errp); >> } >> >> return true; > > ok > >> >> > >> > 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)) { >> >> This changes error messages. >> >> The old ones mention @id. >> >> The new ones don't: >> >> error_setg(errp, "Cryptodev backend is still in use"); >> error_setg(errp, "Host memory backend is still mapped"); >> error_setg(errp, "IOMMUFD backend still has %d users", be->users); >> error_setg(errp, "Throttle group still has multiple references"); >> error_setg(errp, "Deleting can bus devices is not supported");
Spel it CAN, please. >> error_setg(errp, "Deleting main loop is not supported"); >> >> Visible in the diff to tests/qemu-iotests/245 below. >> >> Because this runs within object-del, and @id is the argument provided by >> the user there, the error message is still sufficiently clear, I guess. >> >> If this ever gets used where the object isn't obvious to the user from >> context, the error messages become sub-par. Observation, not a demand. >> >> Please cover this change in the commit message. > > Oh yes, I remember being slightly annoyed at the need to do > this - I was surprised to realize that an Object does not > actually know its own 'id'. The 'id' only exists as the > name of the property in the parent. > > We could add an object_get_id() API which traveres obj->parent > and then iterates over child properties. Annoyingly O(n) time > for that, but perhaps not a big enough problem to worry about ? > At least for error reporting we're probably ok with that time > complexity even if there are 1000 objects registered as children. Could you use object_get_canonical_path_component()? [...]
