On 28/01/2021 14:40, Jan Beulich wrote: > hvm_destroy_all_ioreq_servers(), called from > hvm_domain_relinquish_resources(), invokes relocate_portio_handler(), > which uses d->arch.hvm.io_handler. Defer freeing of this array > accordingly on the error path of hvm_domain_initialise(). > > Similarly rtc_deinit() requires d->arch.hvm.pl_time to still be around, > or else an armed timer structure would get freed, and that timer never > get killed. > > Signed-off-by: Jan Beulich <[email protected]>
Acked-by: Andrew Cooper <[email protected]> > --- > We may want to consider moving the other two XFREE()s later as well, > if only to be on the safe side. Wherever possible, I want to move stuff like this into the idempotent domain_teardown()/_domain_destroy() logic, although I suspect you want this suitable for backport as well? This won't be the last ordering bug we find. Also, I suspect this one is mixed up in the complexity of arch_domain_destroy() which needs aligning carefully between x86 and ARM before it can be switched. > For vRTC I question the calling of check_update_timer() from rtc_init(): > I would consider it more reasonable to do so immediately before the > guest gets first launched, especially if a guest remains paused for a > while after creation. That does look suspect. (And yes - it can take minutes of wallclock time to construct large guests, given the unintentional behaviour introduced by the idle scrubbing logic.) Honestly, its the kind of thing which shouldn't be firing until turned on by HVMLoader. Maybe we're too late to fix that... ~Andrew
