Re: [Xen-devel] [PATCH v3 3/6] libxl: don't try to manipulate json config for stubdomain

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Sat, Jan 26, 2019 at 03:31:14AM +0100, Marek Marczykowski-Górecki wrote:
> Stubdomain do not have it's own config file - its configuration is
> derived from target domains. Do not try to manipulate it when attaching
> PCI device.
> 

So if we add the same configuration to stubdom as well, what would
happen? I guess libxl will try to attach the PCI devices to both the
stubdom and DomU?

> This bug prevented starting HVM with stubdomain and PCI passthrough
> device attached.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> Changes in v3:
>  - skip libxl__dm_check_start too, as stubdomain is guaranteed to be
>    running at this stage already
>  - do not init d_config at all, as it is used only for json manipulation
> ---
>  tools/libxl/libxl_pci.c | 48 ++++++++++++++++++++++++++----------------
>  1 file changed, 30 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> index 1bde537..8d159cf 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -120,10 +120,14 @@ static int libxl__device_pci_add_xenstore(libxl__gc 
> *gc, uint32_t domid, libxl_d
>      libxl_domain_config d_config;
>      libxl_device_pci pcidev_saved;
>      libxl__domain_userdata_lock *lock = NULL;
> +    bool is_stubdomain = libxl_is_stubdom(CTX, domid, NULL);
>  
> -    libxl_domain_config_init(&d_config);
> -    libxl_device_pci_init(&pcidev_saved);
> -    libxl_device_pci_copy(CTX, &pcidev_saved, pcidev);
> +    /* Stubdomain doesn't have own config. */
> +    if (!is_stubdomain) {
> +        libxl_domain_config_init(&d_config);
> +        libxl_device_pci_init(&pcidev_saved);
> +        libxl_device_pci_copy(CTX, &pcidev_saved, pcidev);
> +    }
>  
>      be_path = libxl__domain_device_backend_path(gc, 0, domid, 0,
>                                                  LIBXL__DEVICE_KIND_PCI);
> @@ -152,27 +156,33 @@ static int libxl__device_pci_add_xenstore(libxl__gc 
> *gc, uint32_t domid, libxl_d
>      GCNEW(device);
>      libxl__device_from_pcidev(gc, domid, pcidev, device);
>  
> -    lock = libxl__lock_domain_userdata(gc, domid);
> -    if (!lock) {
> -        rc = ERROR_LOCK_FAIL;
> -        goto out;
> -    }
> +    /* Stubdomain config is derived from its target domain, it doesn't have
> +       its own file */

Although comment style isn't specified in CODING_STYLE, I would like to
fix this to 

    /*
     * Stubdom ...
     * ... own file.
     */

> +    if (!is_stubdomain) {
> +        lock = libxl__lock_domain_userdata(gc, domid);
> +        if (!lock) {
> +            rc = ERROR_LOCK_FAIL;
> +            goto out;
> +        }
>  
> -    rc = libxl__get_domain_configuration(gc, domid, &d_config);
> -    if (rc) goto out;
> +        rc = libxl__get_domain_configuration(gc, domid, &d_config);
> +        if (rc) goto out;
>  
> -    device_add_domain_config(gc, &d_config, &libxl__pcidev_devtype,
> -                             &pcidev_saved);
> +        device_add_domain_config(gc, &d_config, &libxl__pcidev_devtype,
> +                                 &pcidev_saved);
>  
> -    rc = libxl__dm_check_start(gc, &d_config, domid);
> -    if (rc) goto out;
> +        rc = libxl__dm_check_start(gc, &d_config, domid);
> +        if (rc) goto out;
> +    }
>  
>      for (;;) {
>          rc = libxl__xs_transaction_start(gc, &t);
>          if (rc) goto out;
>  
> -        rc = libxl__set_domain_configuration(gc, domid, &d_config);
> -        if (rc) goto out;
> +        if (lock) {
> +            rc = libxl__set_domain_configuration(gc, domid, &d_config);
> +            if (rc) goto out;
> +        }
>  
>          libxl__xs_writev(gc, t, be_path, libxl__xs_kvs_of_flexarray(gc, 
> back));
>  
> @@ -184,8 +194,10 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc, 
> uint32_t domid, libxl_d
>  out:
>      libxl__xs_transaction_abort(gc, &t);
>      if (lock) libxl__unlock_domain_userdata(lock);
> -    libxl_device_pci_dispose(&pcidev_saved);
> -    libxl_domain_config_dispose(&d_config);
> +    if (!is_stubdomain) {
> +        libxl_device_pci_dispose(&pcidev_saved);
> +        libxl_domain_config_dispose(&d_config);
> +    }
>      return rc;
>  }
>  
> -- 
> git-series 0.9.1

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