Re: [PATCH v2 12/12] xen/domain: Allocate d->vcpu[] in domain_create()

["Jan Beulich" <JBeulich@suse.com>]

>>> On 29.08.18 at 12:36, <andrew.cooper3@citrix.com> wrote:
> On 15/08/18 16:18, Jan Beulich wrote:
>>
>>>>> --- a/xen/common/domctl.c
>>>>> +++ b/xen/common/domctl.c
>>>>> @@ -554,16 +554,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> u_domctl)
>>>>>  
>>>>>          ret = -EINVAL;
>>>>>          if ( (d == current->domain) || /* no domain_pause() */
>>>>> -             (max > domain_max_vcpus(d)) )
>>>>> +             (max != d->max_vcpus) )   /* max_vcpus set up in createdomain 
> */
>>>>>              break;
>>>>>  
>>>>> -        /* Until Xenoprof can dynamically grow its vcpu-s array... */
>>>>> -        if ( d->xenoprof )
>>>>> -        {
>>>>> -            ret = -EAGAIN;
>>>>> -            break;
>>>>> -        }
>>>>> -
>>>>>          /* Needed, for example, to ensure writable p.t. state is synced. */
>>>>>          domain_pause(d);
>>>>>  
>>>>> @@ -581,38 +574,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> u_domctl)
>>>>>              }
>>>>>          }
>>>>>  
>>>>> -        /* We cannot reduce maximum VCPUs. */
>>>>> -        ret = -EINVAL;
>>>>> -        if ( (max < d->max_vcpus) && (d->vcpu[max] != NULL) )xc_domain_max_vcpus
>>>>> -            goto maxvcpu_out;
>>>>> -
>>>>> -        /*
>>>>> -         * For now don't allow increasing the vcpu count from a non-zero
>>>>> -         * value: This code and all readers of d->vcpu would otherwise need
>>>>> -         * to be converted to use RCU, but at present there's no tools side
>>>>> -         * code path that would issue such a request.
>>>>> -         */
>>>>> -        ret = -EBUSY;
>>>>> -        if ( (d->max_vcpus > 0) && (max > d->max_vcpus) )
>>>>> -            goto maxvcpu_out;
>>>>> -
>>>>>          ret = -ENOMEM;
>>>>>          online = cpupool_domain_cpumask(d);
>>>>> -        if ( max > d->max_vcpus )
>>>>> -        {
>>>>> -            struct vcpu **vcpus;
>>>>> -
>>>>> -            BUG_ON(d->vcpu != NULL);
>>>>> -            BUG_ON(d->max_vcpus != 0);
>>>>> -
>>>>> -            if ( (vcpus = xzalloc_array(struct vcpu *, max)) == NULL )
>>>>> -                goto maxvcpu_out;
>>>>> -
>>>>> -            /* Install vcpu array /then/ update max_vcpus. */
>>>>> -            d->vcpu = vcpus;
>>>>> -            smp_wmb();
>>>>> -            d->max_vcpus = max;
>>>>> -        }
>>>>>  
>>>>>          for ( i = 0; i < max; i++ )
>>>>>          {
>>>> With all of this dropped, I think the domctl should be renamed. By
>>>> dropping its "max" input at the same time, there would then also
>>>> no longer be a need to check that the value matches what was
>>>> stored during domain creation.
>>> I'm still looking to eventually delete the hypercall, but we need to be
>>> able to clean up all domain/vcpu allocations without calling
>>> complete_domain_destroy, or rearrange the entry logic so
>>> complete_domain_destroy() can be reused for a domain which isn't
>>> currently in the domlist.
>>>
>>> Unfortunately, I think this is going to be fairly complicated, I think.
>> Especially when we expect this to take some time, I think it would
>> be quite helpful for the domctl to actually say what it does until
>> then, rather than retaining its current (then misleading) name.
> 
> Renaming the domctl means renaming xc_domain_max_vcpus(), and the
> python/ocaml stubs, the latter of which does have external users.

This is an option, but the libxc and higher layer functions could as well
be left alone, perhaps with a comment added to the function you name
explaining why its name doesn't match the domctl it uses.

Jan



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