Re: [RFC PATCH] xen: handle domain_shutdown() return values

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.03.2026 15:35, Mykola Kvach wrote:
> On Thu, Mar 19, 2026 at 3:44 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>> On 19.03.2026 13:42, Mykola Kvach wrote:
>>> From: Mykola Kvach <mykola_kvach@xxxxxxxx>
>>>
>>> Propagate domain_shutdown() return codes through the shutdown paths
>>> which can still report errors to their callers, and log explicit
>>> failures in fire-and-forget paths instead of silently discarding the
>>> result.
>>>
>>> This makes the shutdown contract explicit for callers which can report
>>> errors, while preserving observable diagnostics for the remaining
>>> fire-and-forget paths.
>>>
>>> It also fixes MISRA Dir 4.7 and Rule 17.7 violations by ensuring that
>>> the returned status is tested or otherwise used.
>>>
>>> Suggested-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> I don't agree with this. For what you want to do (as per the link below)
>> this is a prereq, but as an independent change I'm not convinced this is
>> needed. Once it is grouped with that other change, it's kind of natural,
>> and hence any Suggested-by: would feel odd.
> 
> I see your point, but I'd still prefer to keep this as a standalone change.
> 
> It is no longer tied to the suspend/resume work, as the changes adding new
> error cases there are gone. What remains is making the existing non-void
> domain_shutdown() contract explicit at its call sites.
> 
> So from my perspective this patch stands on its own for two reasons:
> - it fixes MISRA Dir 4.7 and Rule 17.7 issues by ensuring the returned status
>   is tested or propagated;
> - it avoids leaving latent bugs behind if domain_shutdown() gains additional
>   failure cases in the future, beyond the currently relevant ones.
> 
>>
>> I'm further unconvinced logging is the right course of action in all of
>> the cases. Some may want to be assertions instead?
> 
> That said, I agree the handling likely shouldn't be uniform across all
> callers. I can revisit the fire-and-forget paths and use assertions where
> a non-zero return should be impossible, instead of logging unconditionally.
> 
> If I understand you correctly, then without any additional
> suspend-related error case being introduced, you don't see enough
> value in this as a standalone patch. Is that the right reading?

Not entirely sure. Much would depend on what the description of the change
would say. Addressing Misra concerns, even if just latent ones, is a valid
reason to make such changes, for example.

Jan