[PATCH] nodedev: udev: Hook up virFileWaitForExist to address uevent, race of pci device

Guoyi Tu posted 1 patch 1 year, 1 month ago
Failed in applying to current master (apply log)
src/node_device/node_device_udev.c | 9 +++++++++
1 file changed, 9 insertions(+)
[PATCH] nodedev: udev: Hook up virFileWaitForExist to address uevent, race of pci device
Posted by Guoyi Tu 1 year, 1 month ago
this commit addresses the same issue that as commit 1af45804 does.

the following message is copying from that commit:
   If we find ourselves in the situation that the 'add' uevent has been
   fired earlier than the sysfs tree for a device was created, we should
   use the best-effort approach and give kernel some predetermined amount
   of time, thus waiting for the attributes to be ready rather than
   discarding the device from our device list forever. If those don't appear
   in the given time frame, we need to move on, since libvirt can't wait
   indefinitely.

Signed-off-by: Guoyi Tu <tugy@chinatelecom.cn>
---
  src/node_device/node_device_udev.c | 9 +++++++++
  1 file changed, 9 insertions(+)

diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 1d8486f623..4a1786c21c 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -427,10 +427,19 @@ udevProcessPCI(virNodeDeviceDriverState *driver_state,
      virPCIEDeviceInfo *pci_express = NULL;
      virPCIDevice *pciDev = NULL;
      virPCIDeviceAddress devAddr = { 0 };
+    g_autofree char *linkpath = NULL;
      int ret = -1;
      char *p;
      bool privileged = false;

+    linkpath = g_strdup_printf("%s/config", 
udev_device_get_syspath(device));
+    if (virFileWaitForExists(linkpath, 10, 100) < 0) {
+        virReportSystemError(errno,
+                             _("failed to wait for file '%1$s' to appear"),
+                             linkpath);
+        goto cleanup;
+    }
+
      VIR_WITH_MUTEX_LOCK_GUARD(&driver_state->lock) {
          privileged = driver_state->privileged;
      }
-- 
2.27.0


--
Guoyi
Re: [PATCH] nodedev: udev: Hook up virFileWaitForExist to address uevent, race of pci device
Posted by Martin Kletzander 1 year ago
On Thu, Dec 19, 2024 at 11:07:35PM +0800, Guoyi Tu wrote:
>this commit addresses the same issue that as commit 1af45804 does.
>
>the following message is copying from that commit:
>   If we find ourselves in the situation that the 'add' uevent has been
>   fired earlier than the sysfs tree for a device was created, we should
>   use the best-effort approach and give kernel some predetermined amount
>   of time, thus waiting for the attributes to be ready rather than
>   discarding the device from our device list forever. If those don't appear
>   in the given time frame, we need to move on, since libvirt can't wait
>   indefinitely.
>
>Signed-off-by: Guoyi Tu <tugy@chinatelecom.cn>

Hi and sorry for the delay, this makes sense, I just amended the commit
message and pushed it with my 

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

Thanks for your contribution!
Re: [PATCH] nodedev: udev: Hook up virFileWaitForExist to address uevent, race of pci device
Posted by Guoyi Tu 1 year, 1 month ago
Hi all,

Just a gentle ping to see if there’s any feedback on this patch.
I'm happy to address any comments.Thanks for your time!

On 2024/12/19 23:07, Guoyi Tu wrote:
> this commit addresses the same issue that as commit 1af45804 does.
> 
> the following message is copying from that commit:
>    If we find ourselves in the situation that the 'add' uevent has been
>    fired earlier than the sysfs tree for a device was created, we should
>    use the best-effort approach and give kernel some predetermined amount
>    of time, thus waiting for the attributes to be ready rather than
>    discarding the device from our device list forever. If those don't 
> appear
>    in the given time frame, we need to move on, since libvirt can't wait
>    indefinitely.
> 
> Signed-off-by: Guoyi Tu <tugy@chinatelecom.cn>
> ---
>   src/node_device/node_device_udev.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/src/node_device/node_device_udev.c b/src/node_device/ 
> node_device_udev.c
> index 1d8486f623..4a1786c21c 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -427,10 +427,19 @@ udevProcessPCI(virNodeDeviceDriverState 
> *driver_state,
>       virPCIEDeviceInfo *pci_express = NULL;
>       virPCIDevice *pciDev = NULL;
>       virPCIDeviceAddress devAddr = { 0 };
> +    g_autofree char *linkpath = NULL;
>       int ret = -1;
>       char *p;
>       bool privileged = false;
> 
> +    linkpath = g_strdup_printf("%s/config", 
> udev_device_get_syspath(device));
> +    if (virFileWaitForExists(linkpath, 10, 100) < 0) {
> +        virReportSystemError(errno,
> +                             _("failed to wait for file '%1$s' to 
> appear"),
> +                             linkpath);
> +        goto cleanup;
> +    }
> +
>       VIR_WITH_MUTEX_LOCK_GUARD(&driver_state->lock) {
>           privileged = driver_state->privileged;
>       }