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

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

On Tue, Jan 15, 2019 at 04:36:30PM +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.
> 
> This bug prevented starting HVM with stubdomain and PCI passthrough
> device attached.

I would add a note that the pci device is added to the target domain,
which do have a valid config file.

> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> ---
>  tools/libxl/libxl_pci.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> index 1bde537..e974f55 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -152,14 +152,18 @@ 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 */
> +    if (!libxl_is_stubdom(CTX, domid, NULL)) {
> +        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);

I would put the device_add_domain_config inside of the non-stubdom
case. You could also move the call to libxl_domain_config_init inside
of the non-stubdom case, since d_config is only relevant when there's
no stubdom.

Thanks, Roger.

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