Re: [PATCH v2 07/26] xen/domctl: wrap domain_pause_by_systemcontroller() with MGMT_HYPERCALLS

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.09.2025 09:11, Penny, Zheng wrote:
> [Public]
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: Wednesday, September 10, 2025 11:09 PM
>> To: Penny, Zheng <penny.zheng@xxxxxxx>
>> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; Andrew Cooper
>> <andrew.cooper3@xxxxxxxxxx>; Anthony PERARD <anthony.perard@xxxxxxxxxx>;
>> Orzel, Michal <Michal.Orzel@xxxxxxx>; Julien Grall <julien@xxxxxxx>; Roger 
>> Pau
>> Monné <roger.pau@xxxxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; 
>> xen-
>> devel@xxxxxxxxxxxxxxxxxxxx
>> Subject: Re: [PATCH v2 07/26] xen/domctl: wrap
>> domain_pause_by_systemcontroller() with MGMT_HYPERCALLS
>>
>> On 10.09.2025 09:38, Penny Zheng wrote:
>>> --- a/xen/common/domain.c
>>> +++ b/xen/common/domain.c
>>> @@ -1606,10 +1606,12 @@ static int
>> _domain_pause_by_systemcontroller(struct domain *d, bool sync)
>>>      return 0;
>>>  }
>>>
>>> +#ifdef CONFIG_MGMT_HYPERCALLS
>>>  int domain_pause_by_systemcontroller(struct domain *d)  {
>>>      return _domain_pause_by_systemcontroller(d, true /* sync */);  }
>>> +#endif /* CONFIG_MGMT_HYPERCALLS */
>>>
>>>  int domain_pause_by_systemcontroller_nosync(struct domain *d)
>>>  {
>>
>> I would have ack-ed this if there was only this part, but ...
>>
>>> --- a/xen/common/domctl.c
>>> +++ b/xen/common/domctl.c
>>> @@ -390,11 +390,13 @@ long
>> do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>>>          break;
>>>      }
>>>
>>> +#ifdef CONFIG_MGMT_HYPERCALLS
>>>      case XEN_DOMCTL_pausedomain:
>>>          ret = -EINVAL;
>>>          if ( d != current->domain )
>>>              ret = domain_pause_by_systemcontroller(d);
>>>          break;
>>> +#endif /* CONFIG_MGMT_HYPERCALLS */
>>>
>>>      case XEN_DOMCTL_unpausedomain:
>>>          ret = domain_unpause_by_systemcontroller(d);
>>
>> ... as expressed elsewhere I'm not happy about this one, as it'll need
>> undoing in a later patch of this same series.
>>
> 
> I shall admit that this kind of stub really helps me test MGMT_HYPERCALLS=n 
> for this big serie commit by commit at the very beginning. Otherwise, it 
> could be only disabled (and tested) in the end, and accumulate the mistakes...
> But, as you said, all this transient thing needs to be reversed in the last, 
> and I could accidently missing something and leave dead code...
> As CONFIG_SYSCTL is already a prompt option, then maybe I need to raise a new 
> commit to make it as def_bool again only for this patch serie transiently or 
> just address it in " xen/sysctl: replace CONFIG_SYSCTL with 
> CONFIG_MGMT_DOMCTL " ?

Removing the prompt again (whether in a separate patch or in the renaming one I
wouldn't care much) was what I suggested from the very beginning, but which also
is what faced Stefano's opposition.

Jan