On Wed, Jul 01, 2026 at 03:03:30PM +0200, Markus Armbruster wrote:
Daniel P. Berrangé <berrange@redhat.com> writes:
On Tue, Jun 30, 2026 at 09:34:26AM +0200, Markus Armbruster wrote:
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.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Tested-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
[...]
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 :|