On Fri, Oct 03, 2025 at 14:05:39 +0100, Daniel P. Berrangé wrote:
On Fri, Oct 03, 2025 at 02:58:59PM +0200, Peter Krempa wrote:
On Fri, Oct 03, 2025 at 13:43:31 +0100, Daniel P. Berrangé via Devel wrote:
From: Daniel P. Berrangé <berrange@redhat.com>
The virEventAddHandle/Timeout APIs are unusual in that they do not report errors on failure, because they call through to function callbacks which might be provided externally to libvirt and thus won't be using libvirt's error reporting APIs.
This is a rather unfortunate design characteristic as we can see most callers forgot about this special behaviour and so we are lacking error reporting in many cases.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/libxl/libxl_driver.c | 2 ++ src/logging/log_cleaner.c | 4 ++++ src/logging/log_handler.c | 4 ++++ src/lxc/lxc_controller.c | 5 ++++- src/node_device/node_device_udev.c | 10 +++++++++- src/remote/remote_ssh_helper.c | 10 ++++++++-- src/rpc/virkeepalive.c | 5 ++++- src/rpc/virnetclientstream.c | 2 ++ src/rpc/virnetserverclient.c | 5 ++++- src/rpc/virnetserverservice.c | 2 ++ 10 files changed, 43 insertions(+), 6 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 308c0372aa..6a2e2ab964 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -170,6 +170,7 @@ libxlFDRegisterEventHook(void *priv, info->id = virEventAddHandle(fd, vir_events, libxlFDEventCallback, info, libxlOSEventHookInfoFree); if (info->id < 0) { + VIR_WARN("Failed to add event watch for FD %d", fd); VIR_FREE(info); return -1; } @@ -255,6 +256,7 @@ libxlTimeoutRegisterEventHook(void *priv, info->id = virEventAddTimeout(timeout, libxlTimerCallback, info, libxlOSEventHookInfoFree); if (info->id < 0) { + VIR_WARN("Failed to add event timer"); VIR_FREE(info); return -1; }
IMO adding a warning here will be questionably useful as it's just logged. Does the code that invokes these callbacks report an error that would be correlatable with the warning?
Yeah it isn't ideal, but these two methods are callbacks that are registered with libxl.so, so we don't have a better way to reoprt errors from them AFAIK.
Is it reasonably certain that these won't be called repeatedly if they start failing? So that we don't spam the logs continuously. If yes then consider my R-b also on the warnings.