Re: [PATCH v4] libxl/PCI: Fix PV hotplug & stubdom coldplug

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Jan 12, 2022 at 11:01:42PM -0500, Jason Andryuk wrote:
> commit 0fdb48ffe7a1 "libxl: Make sure devices added by pci-attach are
> reflected in the config" broken PCI hotplug (xl pci-attach) for PV
> domains when it moved libxl__create_pci_backend() later in the function.
> 
> This also broke HVM + stubdom PCI passthrough coldplug.  For that, the
> PCI devices are hotplugged to a running PV stubdom, and then the QEMU
> QMP device_add commands are made to QEMU inside the stubdom.
> 
> A running PV domain calls libxl__wait_for_backend().  With the current
> 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:
> 1) The domain is PV
> 3) The domain is running
> 2) The backend is already present
> 
> 1) xen-pcifront is only used for PV.  It does not load for HVM domains
>    where QEMU is used.
> 
> 2) If the domain is not running (starting), then the frontend state will
>    be Initialising.  xen-pciback waits for the frontend to transition to
>    at Initialised before attempting to connect.  So a wait for a
>    non-running domain is not applicable as the backend will not
>    transition to Connected.
> 
> 3) For presence, num_devs is already used to determine if the backend
>    needs to be created.  Re-use num_devs to determine if the backend
>    wait is necessary.  The wait is necessary to avoid racing with
>    another PCI attachment reconfiguring the front/back or changing to
>    some other state like closing.  If we are creating the backend, then
>    we don't have to worry about the state since it is being created.
> 
> Fixes: 0fdb48ffe7a1 ("libxl: Make sure devices added by pci-attach are
> reflected in the config")
> 
> Signed-off-by: Jason Andryuk <jandryuk@xxxxxxxxx>

That patch is probably good enough for now, even if there is probably need to
rework the function with regards to issue raised in
https://lore.kernel.org/xen-devel/24859.43747.87671.739214@xxxxxxxxxxxxxxxxxxxxxxxx/
in the thread.

Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

Thanks,

-- 
Anthony PERARD