Re: [PATCH v14 3/3] xen/tools: VM forking toolstack side

[Wei Liu <wl@xxxxxxx>]

On Mon, Apr 06, 2020 at 08:20:05AM -0700, Tamas K Lengyel wrote:
> Add necessary bits to implement "xl fork-vm" commands. The command allows the
> user to specify how to launch the device model allowing for a late-launch 
> model
> in which the user can execute the fork without the device model and decide to
> only later launch it.
> 
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxx>
> ---
[...]
>  
> +int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
> +                       libxl__domain_build_state *state,
> +                       uint32_t *domid, bool soft_reset)

It would have been nice if you could split the refactoring out to a
separate patch.

>  static int store_libxl_entry(libxl__gc *gc, uint32_t domid,
>                               libxl_domain_build_info *b_info)
>  {
> @@ -1191,16 +1207,32 @@ static void initiate_domain_create(libxl__egc *egc,
>      ret = libxl__domain_config_setdefault(gc,d_config,domid);
>      if (ret) goto error_out;
>  
> -    ret = libxl__domain_make(gc, d_config, &dcs->build_state, &domid,
> -                             dcs->soft_reset);
> -    if (ret) {
> -        LOGD(ERROR, domid, "cannot make domain: %d", ret);
> +    if ( !d_config->dm_restore_file )
> +    {
> +        ret = libxl__domain_make(gc, d_config, &dcs->build_state, &domid,
> +                                 dcs->soft_reset);
>          dcs->guest_domid = domid;
> +
> +        if (ret) {
> +            LOGD(ERROR, domid, "cannot make domain: %d", ret);
> +            ret = ERROR_FAIL;
> +            goto error_out;
> +        }
> +    } else if ( dcs->guest_domid != INVALID_DOMID ) {

Coding style issue here.

There are quite a few of them.  I won't point them out one by one
though. Let's focus on the interface first.

>  
> +/*
> + * 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?

Wei.