From 878470256ee5498c972258b26748cc8cf5dc809d Mon Sep 17 00:00:00 2001 From: =?utf8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 22 Jul 2024 15:56:03 +0100 Subject: [PATCH] src: report error from failing to add timer/FD watches MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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. Reviewed-by: Peter Krempa Signed-off-by: Daniel P. Berrangé --- 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 | 9 ++++++++- 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, 42 insertions(+), 6 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 308c0372aa..107477250a 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 callback"); VIR_FREE(info); return -1; } diff --git a/src/logging/log_cleaner.c b/src/logging/log_cleaner.c index d247fdf829..7110dfcff6 100644 --- a/src/logging/log_cleaner.c +++ b/src/logging/log_cleaner.c @@ -251,6 +251,10 @@ virLogCleanerInit(virLogHandler *handler) handler->cleanup_log_timer = virEventAddTimeout(CLEANER_LOG_TIMEOUT_MS, virLogCleanerTimer, handler, NULL); + if (handler->cleanup_log_timer < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to add log cleanup timer")); + } return handler->cleanup_log_timer; } diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c index 71517bbbe5..6ad3e33ee8 100644 --- a/src/logging/log_handler.c +++ b/src/logging/log_handler.c @@ -302,6 +302,8 @@ virLogHandlerNewPostExecRestart(virJSONValue *object, virLogHandlerDomainLogFileEvent, handler, NULL)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to add watch on log FD %1$d"), file->pipefd); VIR_DELETE_ELEMENT(handler->files, handler->nfiles - 1, handler->nfiles); goto error; } @@ -386,6 +388,8 @@ virLogHandlerDomainOpenLogFile(virLogHandler *handler, virLogHandlerDomainLogFileEvent, handler, NULL)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to add watch on log FD %1$d"), file->pipefd); VIR_DELETE_ELEMENT(handler->files, handler->nfiles - 1, handler->nfiles); goto error; } diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 3f4c8efdcf..f7776534e8 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -206,8 +206,11 @@ static virLXCController *virLXCControllerNew(const char *name) if ((ctrl->timerShutdown = virEventAddTimeout(-1, virLXCControllerQuitTimer, ctrl, - NULL)) < 0) + NULL)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to add shutdown timer")); goto error; + } cleanup: virLXCControllerDriverFree(driver); diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 30c2ddf568..27e62febe8 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -2318,6 +2318,9 @@ scheduleMdevctlUpdate(udevEventData *data) virEventRemoveTimeout(data->mdevctlTimeout); data->mdevctlTimeout = virEventAddTimeout(100, submitMdevctlUpdate, data, NULL); + if (data->mdevctlTimeout < 0) { + VIR_WARN("Unable to add mdev update timer"); + } } @@ -2609,8 +2612,12 @@ nodeStateInitialize(bool privileged, priv->watch = virEventAddHandle(udev_monitor_get_fd(priv->udev_monitor), VIR_EVENT_HANDLE_READABLE, udevEventHandleCallback, virObjectRef(priv), virObjectUnref); - if (priv->watch == -1) + if (priv->watch == -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to add watch on udev FD %1$d"), + udev_monitor_get_fd(priv->udev_monitor)); goto unlock; + } if (mdevctlEnableMonitor(priv) < 0) goto unlock; diff --git a/src/remote/remote_ssh_helper.c b/src/remote/remote_ssh_helper.c index 2d332a39b6..48896fd559 100644 --- a/src/remote/remote_ssh_helper.c +++ b/src/remote/remote_ssh_helper.c @@ -316,15 +316,21 @@ virRemoteSSHHelperRun(virNetSocket *sock) VIR_EVENT_HANDLE_READABLE, virRemoteSSHHelperEventOnStdin, &proxy, - NULL)) < 0) + NULL)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to add watch on stdin")); goto cleanup; + } if ((proxy.stdoutWatch = virEventAddHandle(STDOUT_FILENO, 0, virRemoteSSHHelperEventOnStdout, &proxy, - NULL)) < 0) + NULL)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to add watch on stdout")); goto cleanup; + } if (virNetSocketAddIOCallback(proxy.sock, VIR_EVENT_HANDLE_READABLE, diff --git a/src/rpc/virkeepalive.c b/src/rpc/virkeepalive.c index d96bd347ad..690bc08b2e 100644 --- a/src/rpc/virkeepalive.c +++ b/src/rpc/virkeepalive.c @@ -276,8 +276,11 @@ virKeepAliveStart(virKeepAlive *ka, ka->intervalStart = now - (ka->interval - timeout); ka->timer = virEventAddTimeout(timeout * 1000, virKeepAliveTimer, ka, virObjectUnref); - if (ka->timer < 0) + if (ka->timer < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to add keepalive timer")); goto cleanup; + } /* the timer now has another reference to this object */ virObjectRef(ka); diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index 98034d737d..380b785869 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -725,6 +725,8 @@ int virNetClientStreamEventAddCallback(virNetClientStream *st, virNetClientStreamEventTimer, st, virObjectUnref)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to add timer to event loop")); virObjectUnref(st); goto cleanup; } diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 355aab4b04..e2967e5e1f 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -396,8 +396,11 @@ virNetServerClientNewInternal(unsigned long long id, client->sockTimer = virEventAddTimeout(-1, virNetServerClientSockTimerFunc, client, NULL); - if (client->sockTimer < 0) + if (client->sockTimer < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to add socket timer")); goto error; + } /* Prepare one for packet receive */ if (!(client->rx = virNetMessageNew(true))) diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c index 682b2091c1..babdedee35 100644 --- a/src/rpc/virnetserverservice.c +++ b/src/rpc/virnetserverservice.c @@ -154,6 +154,8 @@ virNetServerServiceNewSocket(virNetSocket **socks, svc->timer = virEventAddTimeout(-1, virNetServerServiceTimerFunc, svc, virObjectUnref); if (svc->timer < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to add service timer")); virObjectUnref(svc); goto error; } -- 2.47.3