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 4:18 PM Julien Grall <julien.grall.oss@xxxxxxxxx> wrote:
>
> On Fri, 21 Feb 2020 at 22:53, Tamas K Lengyel <tamas.k.lengyel@xxxxxxxxx> 
> wrote:
> >
> > On Fri, Feb 21, 2020 at 3:34 PM Julien Grall <julien@xxxxxxx> wrote:
> > >
> > > Hi Tamas,
> > >
> > > On 21/02/2020 21:35, Tamas K Lengyel wrote:
> > > > 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.
> > >
> > > I don't believe max_vcpus is the only field required here. Most of the
> > > information hold in the structure are required at creation time so the
> > > domain is configured correctly. For instance, on Arm, the version of
> > > interrupt controller can only be configured at creation time. For x86, I
> > > am pretty sure the emuflags have to be correct at creation time as well.
> > >
> > > So it feels weird to me that you only need to copy max_vcpus here.
> > > Because if you are able to fill up the other fields of the structure,
> > > then most likely you have the max_vcpus in hand as well.
> >
> > Look at patch 5 and see how libxl statically define most of these
> > values and why we don't need to copy them.
>
> I looked at patch 5, this is an example of the implementation.
> You limit yourself quite a bit and that's your choice. But I am afraid
> this does not mean the interface with the hypervisor should do the
> same.
>
> [...]
>
> > Julien,
> > you have valid points but at this time I won't be able to refactor
> > this series any more. This patch series was first posted in September,
> > it's now almost March. So at this point I'm just going to say drop
> > this patch and we'll live with the limitation that VM forking only
> > works with single vCPU domains until at some later time someone needs
> > it to work with guests that have more then 1 vCPUs.
>
> To be honest, I don't have a vested interest for the VM forking. So
> the limitation
> of 1 vCPU is fine with me.
>
> Anyone who will want to support more than 1 vCPU with forking will have to
> come up with a proper interface. If you don't want to invest time on it that's
> fine, the rest of the code is still useful to have.

The toolstack can always just decide to do the extra hypercall query
for the max_vcpus and make things work that way. In our usecase we
have single vCPU domains so doing that extra query is something I want
to avoid.

Tamas

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