Re: [PATCH v14 3/3] xen/tools: VM forking toolstack side

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

On Thu, Apr 9, 2020 at 11:11 AM Wei Liu <wl@xxxxxxx> wrote:
>
> On Thu, Apr 09, 2020 at 10:59:55AM -0600, Tamas K Lengyel wrote:
> [...]
> > >
> > > >
> > > > > >
> > > > > > +/*
> > > > > > + * The parent domain is expected to be created with default 
> > > > > > settings for
> > > > > > + * - max_evtch_port
> > > > > > + * - max_grant_frames
> > > > > > + * - max_maptrack_frames
> > > > > > + */
> > > > > > +int libxl_domain_fork_vm(libxl_ctx *ctx, uint32_t pdomid, uint32_t 
> > > > > > max_vcpus, uint32_t *domid)
> > > > > > +{
> > > > > > +    int rc;
> > > > > > +    struct xen_domctl_createdomain create = {0};
> > > > > > +    create.flags |= XEN_DOMCTL_CDF_hvm;
> > > > > > +    create.flags |= XEN_DOMCTL_CDF_hap;
> > > > > > +    create.flags |= XEN_DOMCTL_CDF_oos_off;
> > > > > > +    create.arch.emulation_flags = (XEN_X86_EMU_ALL & 
> > > > > > ~XEN_X86_EMU_VPCI);
> > > > > > +    create.ssidref = SECINITSID_DOMU;
> > > > > > +    create.max_vcpus = max_vcpus;
> > > > >
> > > > >
> > > > > Some questions:
> > > > >
> > > > > Why does the caller need to specify the number of vcpus?
> > > > >
> > > > > Since the parent (source) domain is around, can you retrieve its 
> > > > > domain
> > > > > configuration such that you know its max_vcpus and other information
> > > > > including max_evtchn_port and maptrack frames?
> > > >
> > > > Because we want to avoid having to issue an extra hypercall for these.
> > > > Normally these pieces of information will be available for the user
> > > > and won't change, so we save time by not querying for it every time a
> > > > fork is created. Remember, we might be creating thousands of forks in
> > > > a very short time, so those extra hypercalls add up.
> > >
> > > I see. Speed is a big concern to you.
> > >
> > > What I was referring to doesn't require issuing hypercalls but requires
> > > calling libxl_retrieve_domain_configuration. That's likely to be even
> > > slower than making a hypercall.
> >
> > Right. We only want to parse the domain config if the device model is
> > being launched.
> >
> > >
> > > I'm afraid the current incarnation of libxl_domain_fork_vm cannot become
> > > supported (as in Xen's support statement) going forward, because it is
> > > leaky.
> >
> > What do you mean by leaky?
>
> It requires the caller to specify the number of max_vcpus. The reason
> for doing that is because you want to avoid extra hypercall(s). There is
> nothing that stops someone from coming along in the future claiming some
> other parameters are required because of the same concern you have
> today. It is an optimisation, not a must-have in terms of functionality.
>
> To me the number shouldn't be specified by the caller in the first
> place. It is like forking a process somehow requires you to specify how
> many threads it will have.

I agree. It's not how I wanted to have the interface work but
unfortunately this was the least "ugly" of the possible solutions
given the circumstances.

> >
> > >
> > > I can see two solutions: 1. batch forking to reduce the number of
> > > queries needed; 2. make a proper domctl which duplicates the source
> > > domain structure inside Xen.
> >
> > I've attempted to do that by extending the domain create hypercall so
> > that this information can be copied in the hypervisor. That approach
> > was very unpopular.
> >
>
> Sigh. Sorry I haven't had the chance to read previous discussions.
> -ETIME.

Sigh indeed.

Tamas