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

[Julien Grall <julien.grall.oss@xxxxxxxxx>]

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.

Cheers,

-- 
Julien Grall

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