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

[Wei Liu <wl@xxxxxxx>]

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

Wei.