Re: [Xen-devel] [PATCH v2 00/20] VM forking

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Jan 08, 2020 at 08:32:22AM -0700, Tamas K Lengyel wrote:
> On Wed, Jan 8, 2020 at 8:08 AM Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
> >
> > On Tue, Dec 31, 2019 at 09:36:01AM -0700, Tamas K Lengyel wrote:
> > > On Tue, Dec 31, 2019 at 9:08 AM Tamas K Lengyel <tamas@xxxxxxxxxxxxx> 
> > > wrote:
> > > >
> > > > On Tue, Dec 31, 2019 at 8:11 AM Roger Pau Monné <roger.pau@xxxxxxxxxx> 
> > > > wrote:
> > > > >
> > > > > On Tue, Dec 31, 2019 at 08:00:17AM -0700, Tamas K Lengyel wrote:
> > > > > > On Tue, Dec 31, 2019 at 3:40 AM Roger Pau Monné 
> > > > > > <roger.pau@xxxxxxxxxx> wrote:
> > > > > > >
> > > > > > > On Mon, Dec 30, 2019 at 05:37:38PM -0700, Tamas K Lengyel wrote:
> > > > > > > > On Mon, Dec 30, 2019 at 5:20 PM Julien Grall 
> > > > > > > > <julien.grall@xxxxxxxxx> wrote:
> > > > > > > > >
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > On Mon, 30 Dec 2019, 20:49 Tamas K Lengyel, 
> > > > > > > > > <tamas@xxxxxxxxxxxxx> wrote:
> > > > > > > > >>
> > > > > > > > >> On Mon, Dec 30, 2019 at 11:43 AM Julien Grall 
> > > > > > > > >> <julien@xxxxxxx> wrote:
> > > > > > > > >> But keep in mind that the "fork-vm" command even with this 
> > > > > > > > >> update
> > > > > > > > >> would still not produce for you a "fully functional" VM on 
> > > > > > > > >> its own.
> > > > > > > > >> The user still has to produce a new VM config file, create 
> > > > > > > > >> the new
> > > > > > > > >> disk, save the QEMU state, etc.
> > > > > > >
> > > > > > > IMO the default behavior of the fork command should be to leave 
> > > > > > > the
> > > > > > > original VM paused, so that you can continue using the same disk 
> > > > > > > and
> > > > > > > network config in the fork and you won't need to pass a new config
> > > > > > > file.
> > > > > > >
> > > > > > > As Julien already said, maybe I wasn't clear in my previous 
> > > > > > > replies:
> > > > > > > I'm not asking you to implement all this, it's fine if the
> > > > > > > implementation of the fork-vm xl command requires you to pass 
> > > > > > > certain
> > > > > > > options, and that the default behavior is not implemented.
> > > > > > >
> > > > > > > We need an interface that's sane, and that's designed to be easy 
> > > > > > > and
> > > > > > > comprehensive to use, not an interface built around what's 
> > > > > > > currently
> > > > > > > implemented.
> > > > > >
> > > > > > OK, so I think that would look like "xl fork-vm <parent_domid>" with
> > > > > > additional options for things like name, disk, vlan, or a completely
> > > > > > new config, all of which are currently not implemented, + an
> > > > > > additional option to not launch QEMU at all, which would be the only
> > > > > > one currently working. Also keeping the separate "xl fork-launch-dm"
> > > > > > as is. Is that what we are talking about?
> > > > >
> > > > > I think fork-launch-vm should just be an option of fork-vm (ie:
> > > > > --launch-dm-only or some such). I don't think there's a reason to have
> > > > > a separate top-level command to just launch the device model.
> > > >
> > > > It's just that the fork-launch-dm needs the domid of the fork, while
> > > > the fork-vm needs the parent's domid. But I guess we can interpret the
> > > > "domid" required input differently depending on which sub-option is
> > > > specified for the command. Let's see how it pans out.
> > >
> > > How does the following look for the interface?
> > >
> > >     { "fork-vm",
> > >       &main_fork_vm, 0, 1,
> > >       "Fork a domain from the running parent domid",
> > >       "[options] <Domid>",
> > >       "-h                           Print this help.\n"
> > >       "-N <name>                    Assign name to VM fork.\n"
> > >       "-D <disk>                    Assign disk to VM fork.\n"
> > >       "-B <bridge                   Assign bridge to VM fork.\n"
> > >       "-V <vlan>                    Assign vlan to VM fork.\n"
> >
> > IMO I think the name of fork is the only useful option. Being able to
> > assign disks or bridges from the command line seems quite complicated.
> > What about VMs with multiple disks? Or VMs with multiple nics on
> > different bridges?
> >
> > I think it's easier for both the implementation and the user to just
> > use a config file in that case.
> 
> I agree but it sounded to me you guys wanted to have a "complete"
> interface even if it's unimplemented. This is what a complete
> interface would look to me.

I would add those options afterwards if there's a need for them. I was
mainly concerned about introducing a top level command (ie: fork-vm)
that would require calling other commands in order to get a functional
fork. I'm not so concerned about having all the possible options
listed now, as long as the default behavior of fork-vm is something
sane that produces a working fork, even if not fully implemented at
this stage.

> >
> > >       "-C <config>                  Use config file for VM fork.\n"
> > >       "-Q <qemu-save-file>          Use qemu save file for VM fork.\n"
> > >       "--launch-dm  <yes|no|late>   Launch device model (QEMU) for VM 
> > > fork.\n"
> > >       "--fork-reset                 Reset VM fork.\n"
> > >       "-p                           Do not unpause VMs after fork."
> >
> > I think the default behaviour should be to leave the original VM
> > paused and the forked one running, and hence this should be:
> 
> That is the default. I guess the text saying VMs was not correctly
> worded, it just means don't unpause fork after it's created. The
> parent remains always paused.

Ack.

> >
> >         "-p                           Leave forked VM paused."
> >         "-u                           Leave parent VM unpaused."
> 
> But you shouldn't unpause the parent VM at all. It should remain
> paused as long as there are forks running that were split from it.
> Unpausing it will lead to subtle and unexplainable crashes in the fork
> since the fork now will use pages that are from a different execution
> path. Technically in the future it would be possible to unpause the VM
> but it requires to fully populate the pagetables in all forks made
> from it with mem_shared entries and deduplicate to the forks all the
> pages that's can't be be shared.

Oh, OK, since I didn't look at the implementation yet I assumed that
the parent would also be switched to trap on memory writes, so that
you could duplicate the pages before the parent writes to them, and
hence the parent could be left running.

Anyway, let's forget about the "leave parent unpaused" option then.

> That was what I originally tried to
> do but it was extremely slow, hence the lazy-population of the
> pagetable in the forks.
> 
> >
> > >       "-h                           Print this help.\n"
> > >       "-d                           Enable debug messages.\n"
> > >     },
> > >
> > > Currently the parts that are implemented would look like:
> > > xl fork-vm -p --launch-dm no <parent_domid>
> > > xl fork-vm -p --launch-dm late -C <config> -Q <qemu-save-file> 
> > > <fork_domid>
> >
> > Why do you need a config file for launching the Qemu device model?
> > Doesn't the save-file contain all the information?
> 
> The config is used to populate xenstore, not just for QEMU. The QEMU
> save file doesn't contain the xl config. This is not a full VM save
> file, it is only the QEMU state that gets dumped with
> xen-save-devices-state.

TBH I think it would be easier to have something like my proposal
below, where you tell xl the parent and the forked VM names and xl
does the rest. Even better would be to not have to tell xl the parent
VM name (since I guess this is already tracked internally somewhere?).

Anyway, I'm not going to insist on this but the workflow of the Qemu
forking seems to not be very user friendly unless you know exactly how
to use it.

> 
> >
> > I think you also need something like:
> >
> > # xl fork-vm --launch-dm late <parent_domid> <fork_domid>
> >
> > So that a user doesn't need to pass a qemu-save-file?
> 
> This doesn't make much sense to me. To launch QEMU you need the config
> file to wire things up correctly. Like in order to launch QEMU you
> need to tell it the name of the VM, disk path, etc. that are all
> contained in the config.

You could get all this information from the parent VM, IIRC libxl has
a json version of the config. For example for migration there's no
need to pass any config file, since the incoming VM can be recreated
from the data in the source VM.

Thanks, Roger.

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