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

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

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.

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

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

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

>
> Can you also list the command used to get the Qemu save-file from the
> parent? (just for completeness purposes).

It's explained in the cover letter. You connect to the QEMU socket and
issue the xen-save-devices-state QMP command.

Tamas

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