On Wed, Jan 28, 2026 at 03:34:11PM +0100, Jan Beulich wrote: > On 28.01.2026 14:33, Roger Pau Monné wrote: > > On Wed, Jan 28, 2026 at 12:44:26PM +0000, Andrew Cooper wrote: > >> In principle we could assert that it's already NULL in _domain_destroy() > >> which might help catch an error if it gets set early but the domain > >> destroyed before getting into the domlist, but that seems like a rather > >> slim case. > > > > Given my understanding of the logic in the XENMEM_ hypercalls, I think > > toolstack can still target domains in the process of being destroyed, > > at which point we need to keep a final cleanup instance > > _domain_destroy(), or otherwise adjust XENMEM_populate_physmap > > hypercall (and others?) so they can't target dying domains. > > Considering that these requests will fail for dying domains because of the > check in assign_pages(), it may indeed make sense to have another earlier > check for the purposes here. Otoh doing this early may not buy us very > much, as the domain may become "dying" immediately after the check. Whereas > switching stash_allocation()'s if() to > > if ( d->pending_scrub || d->is_dying ) > > looks like it might do.
Oh, I didn't notice the check in assign_pages(). I think a check in stash_allocation() together with a call to domain_pending_scrub_free() in domain_teardown() (which is after setting d->is_dying) should be enough to ensure the field is clear when reaching _domain_destroy(). The call to domain_pending_scrub_free() in _domain_destroy() can be replaced with an ASSERT(!d->pending_scrub) then. Thanks, Roger.
