On Wed, Jul 01, 2026 at 03:03:30PM +0200, Markus Armbruster wrote: > 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()?
Yes ! 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 :|
