[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/3] xen/domain: Introduce domain_teardown()
On 22.12.2020 12:46, Andrew Cooper wrote: > On 22/12/2020 10:35, Jan Beulich wrote: >> On 21.12.2020 19:14, Andrew Cooper wrote: >>> --- a/xen/common/domain.c >>> +++ b/xen/common/domain.c >>> @@ -273,6 +273,59 @@ static int __init parse_extra_guest_irqs(const char *s) >>> custom_param("extra_guest_irqs", parse_extra_guest_irqs); >>> >>> /* >>> + * Release resources held by a domain. There may or may not be live >>> + * references to the domain, and it may or may not be fully constructed. >>> + * >>> + * d->is_dying differing between DOMDYING_dying and DOMDYING_dead can be >>> used >>> + * to determine if live references to the domain exist, and also whether >>> + * continuations are permitted. >>> + * >>> + * If d->is_dying is DOMDYING_dead, this must not return non-zero. >>> + */ >>> +static int domain_teardown(struct domain *d) >>> +{ >>> + BUG_ON(!d->is_dying); >>> + >>> + /* >>> + * This hypercall can take minutes of wallclock time to complete. This >>> + * logic implements a co-routine, stashing state in struct domain >>> across >>> + * hypercall continuation boundaries. >>> + */ >>> + switch ( d->teardown.val ) >>> + { >>> + /* >>> + * Record the current progress. Subsequent hypercall continuations >>> + * will logically restart work from this point. >>> + * >>> + * PROGRESS() markers must not be in the middle of loops. The loop >>> + * variable isn't preserved across a continuation. >>> + * >>> + * To avoid redundant work, there should be a marker before each >>> + * function which may return -ERESTART. >>> + */ >>> +#define PROGRESS(x) \ >>> + d->teardown.val = PROG_ ## x; \ >>> + /* Fallthrough */ \ >>> + case PROG_ ## x >>> + >>> + enum { >>> + PROG_done = 1, >>> + }; >>> + >>> + case 0: >>> + PROGRESS(done): >>> + break; >>> + >>> +#undef PROGRESS >>> + >>> + default: >>> + BUG(); >>> + } >>> + >>> + return 0; >>> +} >> While as an initial step this may be okay, I envision ordering issues >> with domain_relinquish_resources() - there may be per-arch things that >> want doing before certain common ones, and others which should only >> be done when certain common teardown was already performed. Therefore >> rather than this being a "common equivalent of >> domain_relinquish_resources()", I'd rather see (and call) it a >> (designated) replacement, where individual per-arch functions would >> get called at appropriate times. > > Over time, I do expect it to replace domain_relinquish_resources(). I'm > not sure if that will take the form of a specific arch_domain_teardown() > call (and reserving some space in teardown.val for arch use), or whether > it will be a load of possibly-CONFIG'd stubs. I'd consider helpful if you could slightly re-word the description then. Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |