[PATCH] nodedev: ignore EINVAL from libudev in udevEventHandleThread

From: Christian Ehrhardt <christian.ehrhardt@canonical.com> Certiain udev entries might be of a size that makes libudev emit EINVAL which right now leads to udevEventHandleThread exiting. Due to no more handling events other elements of libvirt will start pushing for events to be consumed which never happens causing a busy loop burning a cpu without any gain. After evaluation of the root cause of the example case discussed in in #245 and a test run ignoring EINVAL it was considered safe to add EINVAL to the ignored errnos to not exit udevEventHandleThread giving it more resilience. Fixes: #245 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/node_device/node_device_udev.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 07c10f0d88..21fbcc2dca 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1863,10 +1863,12 @@ udevEventHandleThread(void *opaque G_GNUC_UNUSED) } /* POSIX allows both EAGAIN and EWOULDBLOCK to be used - * interchangeably when the read would block or timeout was fired + * interchangeably when the read would block or timeout was fired. + * EINVAL might happen on too large udev entries, ignore those for + * the robustness of udevEventHandleThread. */ VIR_WARNINGS_NO_WLOGICALOP_EQUAL_EXPR - if (errno != EAGAIN && errno != EWOULDBLOCK) { + if (errno != EAGAIN && errno != EWOULDBLOCK && errno != EINVAL) { VIR_WARNINGS_RESET virReportSystemError(errno, "%s", _("failed to receive device from udev " -- 2.38.0

On Thu, Oct 13, 2022 at 08:05:41AM +0200, christian.ehrhardt@canonical.com wrote:
From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Certiain udev entries might be of a size that makes libudev emit EINVAL which right now leads to udevEventHandleThread exiting. Due to no more handling events other elements of libvirt will start pushing for events to be consumed which never happens causing a busy loop burning a cpu without any gain.
After evaluation of the root cause of the example case discussed in in #245 and a test run ignoring EINVAL it was considered safe to add EINVAL to the ignored errnos to not exit udevEventHandleThread giving it more resilience.
Fixes: #245
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/node_device/node_device_udev.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Thu, Oct 13, 2022 at 08:05:41AM +0200, christian.ehrhardt@canonical.com wrote:
From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Certiain udev entries might be of a size that makes libudev emit EINVAL which right now leads to udevEventHandleThread exiting. Due to no more handling events other elements of libvirt will start pushing for events to be consumed which never happens causing a busy loop burning a cpu without any gain.
After evaluation of the root cause of the example case discussed in in #245 and a test run ignoring EINVAL it was considered safe to add EINVAL to the ignored errnos to not exit udevEventHandleThread giving it more resilience.
Fixes: #245
Please always use full URLs instead of number references to have clickable links from running git history in a terminal. Regards, Erik

On Thu, Oct 13, 2022 at 10:06 AM Erik Skultety <eskultet@redhat.com> wrote:
On Thu, Oct 13, 2022 at 08:05:41AM +0200, christian.ehrhardt@canonical.com wrote:
From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Certiain udev entries might be of a size that makes libudev emit EINVAL which right now leads to udevEventHandleThread exiting. Due to no more handling events other elements of libvirt will start pushing for events to be consumed which never happens causing a busy loop burning a cpu without any gain.
After evaluation of the root cause of the example case discussed in in #245 and a test run ignoring EINVAL it was considered safe to add EINVAL to the ignored errnos to not exit udevEventHandleThread giving it more resilience.
Fixes: #245
Please always use full URLs instead of number references to have clickable links from running git history in a terminal.
Sure, done Sent a v2 with the reviewed-by and the fix as URL I also did file a systemd issue and referenced it from our issue. *sigh* and now also a v3 as I saw a typo too late
Regards, Erik
-- Christian Ehrhardt Senior Staff Engineer, Ubuntu Server Canonical Ltd
participants (4)
-
Christian Ehrhardt
-
christian.ehrhardt@canonical.com
-
Daniel P. Berrangé
-
Erik Skultety