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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.08.18 at 12:01, <andrew.cooper3@xxxxxxxxxx> wrote:
> @@ -423,6 +436,11 @@ struct domain *domain_create(domid_t domid,
>  
>      sched_destroy_domain(d);
>  
> +    if ( d->max_vcpus )
> +    {
> +        d->max_vcpus = 0;
> +        XFREE(d->vcpu);
> +    }
>      if ( init_status & INIT_arch )
>          arch_domain_destroy(d);

I'm not sure it is a good idea to free the vcpus this early, in particular
before arch_domain_destroy().

> --- 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) )
> -            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.

Jan



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