On 03.12.2025 13:13, Mykola Kvach wrote: > Hi Jan, > > Thank you for the review. > > On Wed, Dec 3, 2025 at 12:11 PM Jan Beulich <[email protected]> wrote: >> >> On 03.12.2025 10:57, Mykola Kvach wrote: >>> --- a/xen/common/domain.c >>> +++ b/xen/common/domain.c >>> @@ -26,6 +26,7 @@ >>> #include <xen/hypercall.h> >>> #include <xen/delay.h> >>> #include <xen/shutdown.h> >>> +#include <xen/suspend.h> >>> #include <xen/percpu.h> >>> #include <xen/multicall.h> >>> #include <xen/rcupdate.h> >>> @@ -1363,6 +1364,9 @@ void domain_resume(struct domain *d) >>> >>> spin_lock(&d->shutdown_lock); >>> >>> + if ( arch_domain_resume(d) ) >>> + goto fail; >> >> In case I didn't ask before: You're after a boolean result here, yet ... >> >>> --- /dev/null >>> +++ b/xen/include/xen/suspend.h >>> @@ -0,0 +1,25 @@ >>> +/* SPDX-License-Identifier: GPL-2.0-only */ >>> + >>> +#ifndef XEN_SUSPEND_H >>> +#define XEN_SUSPEND_H >>> + >>> +#if __has_include(<asm/suspend.h>) >>> +#include <asm/suspend.h> >>> +#else >>> +static inline int arch_domain_resume(struct domain *d) >>> +{ >>> + return 0; >>> +} >>> +#endif >> >> ... int is being returned. Why? > > Would you prefer I switch arch_domain_resume() to bool, or is keeping > int acceptable? > > I lean toward keeping int: the generic call site is shared by all arches, > so future arches could inspect/handle specific error codes,
It's an arch hook, so the arch provides the error code. It's common code which may want to inspect it. > and this matches > other arch hooks (e.g. arch_domain_teardown()) that are only checked for > "rc != 0" before bailing. And it's questionable there as well why they would return an error code if the sole caller cares about a boolean outcome only. > With int, I'll store the result and gate the rest: > > rc = arch_domain_resume(d); > if (rc) > goto fail; > > If int works for you, I’ll keep it; otherwise I can flip to bool. If you want to stick to int, please justify doing so in the description. Jan
