[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v14 3/3] xen/tools: VM forking toolstack side
On Fri, Apr 10, 2020 at 10:19 AM Wei Liu <wl@xxxxxxx> wrote: > > On Fri, Apr 10, 2020 at 10:05:50AM -0600, Tamas K Lengyel wrote: > > On Thu, Apr 9, 2020 at 11:30 AM Tamas K Lengyel > > <tamas.k.lengyel@xxxxxxxxx> wrote: > > > > > > 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. > > > > By the way, it would be trivial to query the parent in case max_vcpus > > is not provided by the user. But I would still like to keep the option > > available to skip that step if the number is known already. Let me > > know if you would like me to add that. > > I'm still waiting for Ian and Anthony to chime in to see whether they > agree to keep this interface unstable. > > At the end of the day, if it is unstable, I don't really care that much. > I'm happy to let you (the developer and user) have more say in that > case. I will instead divert my (limited) time to code that your patch > touches which is also used by other stable functions. SGTM Thanks, Tamas
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |