Re: [Xen-devel] [PATCH v9 3/5] xen/mem_sharing: VM forking

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.02.2020 13:39, Roger Pau Monné wrote:
> On Fri, Feb 21, 2020 at 10:49:21AM -0800, Tamas K Lengyel wrote:
>> +static int mem_sharing_fork(struct domain *d, struct domain *cd)
>> +{
>> +    int rc = -EINVAL;
>> +
>> +    if ( !cd->controller_pause_count )
>> +        return rc;
> 
> -EBUSY might be better here.
> 
>> +
>> +    /*
>> +     * We only want to get and pause the parent once, not each time this
>> +     * operation is restarted due to preemption.
>> +     */
>> +    if ( !cd->parent_paused )
>> +    {
>> +        ASSERT(get_domain(d));
> 
> We are trying to avoid such constructs, instead I suggest:
> 
> if ( !get_domain(parent) )
> {
>     ASSERT_UNREACHABLE();
>     return -EBUSY;
> }

But isn't the ASSERT() here wrong anyway? I.e. what is it that
guarantees that d hasn't gone away? If it's the caller of this
function, then wouldn't it be get_knownalive_domain() that
wants using here?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel