Re: [PATCH] libxl: Fix stubdom PCI passthrough

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.08.2021 00:17, Jason Andryuk wrote:
> commit 0fdb48ffe7a1 "libxl: Make sure devices added by pci-attach are
> reflected in the config"

This should be in a Fixes: tag; whether you fully spell out the
reference here or instead refer to that tag would by up to you.

> broken stubdom PCI passthrough when it moved
> libxl__create_pci_backend later in the function.  xl pci-attach for a
> running PV domain may also have been broken, but that was not verified.
> 
> The stubdomain is running (!starting) and PV, so it calls
> libxl__wait_for_backend.  With the new placement of
> libxl__create_pci_backend, the path does not exist and the call
> immediately fails.
> libxl: error: libxl_device.c:1388:libxl__wait_for_backend: Backend 
> /local/domain/0/backend/pci/43/0 does not exist
> libxl: error: libxl_pci.c:1764:device_pci_add_done: Domain 
> 42:libxl__device_pci_add failed for PCI device 0:2:0.0 (rc -3)
> libxl: error: libxl_create.c:1857:domcreate_attach_devices: Domain 42:unable 
> to add pci devices
> 
> The wait is only relevant when the backend is already present.  Use the
> read on num_devs to decide whether the backend exists and therefore the
> wait is needed.

But the presence of the node is not an indication of the backend existing,
is it? libxl__device_pci_add_xenstore() itself writes the node a few lines
down from your change, so upon processing a 2nd device the function would
still behave as it does now.

Jan

>  This restores starting an HVM w/ linux stubdom and PCI
> passthrough.
> 
> Signed-off-by: Jason Andryuk <jandryuk@xxxxxxxxx>
> ---
> Looks like this should be backported to 4.15, but I have not tested.
> ---
>  tools/libs/light/libxl_pci.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
> index 1a1c263080..19daf1d4ee 100644
> --- a/tools/libs/light/libxl_pci.c
> +++ b/tools/libs/light/libxl_pci.c
> @@ -157,8 +157,10 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc,
>      if (domtype == LIBXL_DOMAIN_TYPE_INVALID)
>          return ERROR_FAIL;
>  
> -    if (!starting && domtype == LIBXL_DOMAIN_TYPE_PV) {
> -        if (libxl__wait_for_backend(gc, be_path, GCSPRINTF("%d", 
> XenbusStateConnected)) < 0)
> +    /* wait is only needed if the backend already exists (num_devs != NULL) 
> */
> +    if (num_devs && !starting && domtype == LIBXL_DOMAIN_TYPE_PV) {
> +        if (libxl__wait_for_backend(gc, be_path,
> +                                    GCSPRINTF("%d", XenbusStateConnected)) < 
> 0)
>              return ERROR_FAIL;
>      }
>  
>