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 <jandr...@gmail.com>
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.739...@mariner.uk.xensource.com/
in the thread.
Reviewed-by: Anthony PERARD <anthony.per...@citrix.com>
Thanks,
--
Anthony PERARD
