Re: [Xen-devel] [PATCH v9 2/5] xen: add parent_domid field to createdomain domctl

[Tamas K Lengyel <tamas.k.lengyel@xxxxxxxxx>]

On Fri, Feb 21, 2020 at 2:03 PM Julien Grall <julien@xxxxxxx> wrote:
>
> Hi Tamas,
>
> On 21/02/2020 18:49, Tamas K Lengyel wrote:
> > When creating a domain that will be used as a VM fork some information is
> > required to set things up properly, like the max_vcpus count. Instead of the
> > toolstack having to gather this information for each fork in a separate
> > hypercall we can just include the parent domain's id in the createdomain 
> > domctl
> > so that Xen can copy the setting without the extra toolstack queries.
>
> It is not entirely clear why you only want to copy max_vcpus. From my
> understanding,  when you are going to fork a domain you will want the
> domain to be nearly identical. So how do you decide what to copy?

All parameters of the parent domain need to be copied, but during
createdomain domctl only max_vcpus is required. The rest are copied
during XENMEM_sharing_op_fork.

>
> >
> > Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxx>
> > ---
> >   xen/common/domctl.c         | 14 ++++++++++++++
> >   xen/include/public/domctl.h |  3 ++-
> >   2 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> > index a69b3b59a8..22aceb3860 100644
> > --- a/xen/common/domctl.c
> > +++ b/xen/common/domctl.c
> > @@ -489,6 +489,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> > u_domctl)
> >       case XEN_DOMCTL_createdomain:
> >       {
> >           domid_t        dom;
> > +        domid_t        parent_dom;
> >           static domid_t rover = 0;
> >
> >           dom = op->domain;
> > @@ -515,6 +516,19 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> > u_domctl)
> >               rover = dom;
> >           }
> >
> > +        parent_dom = op->u.createdomain.parent_domid;
> > +        if ( parent_dom )
>
> I would rather avoid to assume that parent_dom will not be 0 for a few
> reasons:
>     1) Most of Xen (if not all) now avoid to assume that dom0->domain_id
> == 0.
>     2) I can see usecases where it we may want to recreate dom0 setup.

That's an interesting thought, I don't expect that will be a usecase
but it's interesting. Currently I don't think it would work, so I
consider that to be out-of-scope.

>
> So we should consider a different value to indicate whether we want to
> clone from a domain. Maybe by setting bit 16 of the parent_domid?

I can add a XEN_DOMCTL_CDF_fork flag, but I think that's an overkill.

>
> > +        {
> > +            struct domain *pd = rcu_lock_domain_by_id(parent_dom);
> > +
> > +            ret = -EINVAL;
> > +            if ( !pd )
> > +                break;
> > +
> > +            op->u.createdomain.max_vcpus = pd->max_vcpus;
> > +            rcu_unlock_domain(pd);
> > +        }
> > +
> >           d = domain_create(dom, &op->u.createdomain, false);
> >           if ( IS_ERR(d) )
> >           {
> > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> > index fec6f6fdd1..251cc40ef6 100644
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -38,7 +38,7 @@
> >   #include "hvm/save.h"
> >   #include "memory.h"
> >
> > -#define XEN_DOMCTL_INTERFACE_VERSION 0x00000012
> > +#define XEN_DOMCTL_INTERFACE_VERSION 0x00000013
> >
> >   /*
> >    * NB. xen_domctl.domain is an IN/OUT parameter for this operation.
> > @@ -92,6 +92,7 @@ struct xen_domctl_createdomain {
> >       uint32_t max_evtchn_port;
> >       int32_t max_grant_frames;
> >       int32_t max_maptrack_frames;
> > +    domid_t parent_domid;
>
> By just looking at the name, it is not clear what the field is for. It
> also suggest that one domain will be linked to the other. But this is
> not the case here. I would recommend to add a comment explaining how
> this is used by Xen.

No, during createdomain the domains won't get linked. Only once the
XENMEM_sharing_op_fork finishes do the domains get linked. I explain
this in the patch message, I can copy that as a comment into the
header if you prefer.

Tamas

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