On Tue, Aug 18, 2020 at 09:47:59AM -0500, Jonathon Jongsma wrote:
> We need to peridocally query mdevctl for changes to device definitions
> since an administrator can define new devices with mdevctl outside of
> libvirt.
>
> In order to do this, a new thread is created to handle the querying. In
> the future, mdevctl may add a way to signal device add / remove via
> events, but for now we resort to a bit of a workaround: monitoring the
> mdevctl config directories for changes to files. When a change is
> detected, we query mdevctl and update our device list.
>
> Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> ---
> src/node_device/node_device_udev.c | 182 +++++++++++++++++++++++++++++
> 1 file changed, 182 insertions(+)
>
> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
> index e06584a3dc..a85418e549 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -41,6 +41,7 @@
> #include "virnetdev.h"
> #include "virmdev.h"
> #include "virutil.h"
> +#include <gio/gio.h>
>
> #include "configmake.h"
>
> @@ -66,6 +67,11 @@ struct _udevEventData {
> virCond threadCond;
> bool threadQuit;
> bool dataReady;
> +
> + virThread mdevctlThread;
> + virCond mdevctlCond;
> + bool mdevctlReady;
> + GList *mdevctl_monitors;
> };
>
> static virClassPtr udevEventDataClass;
> @@ -85,8 +91,10 @@ udevEventDataDispose(void *obj)
> udev = udev_monitor_get_udev(priv->udev_monitor);
> udev_monitor_unref(priv->udev_monitor);
> udev_unref(udev);
> + g_list_free_full(priv->mdevctl_monitors, g_object_unref);
>
> virCondDestroy(&priv->threadCond);
> + virCondDestroy(&priv->mdevctlCond);
> }
>
>
> @@ -117,6 +125,11 @@ udevEventDataNew(void)
> return NULL;
> }
>
> + if (virCondInit(&ret->mdevctlCond) < 0) {
> + virObjectUnref(ret);
> + return NULL;
> + }
> +
> ret->watch = -1;
> return ret;
> }
> @@ -1520,6 +1533,7 @@ nodeStateCleanup(void)
> virObjectLock(priv);
> priv->threadQuit = true;
> virCondSignal(&priv->threadCond);
> + virCondSignal(&priv->mdevctlCond);
> virObjectUnlock(priv);
> virThreadJoin(&priv->th);
> }
> @@ -1601,6 +1615,40 @@ udevEventMonitorSanityCheck(udevEventDataPtr priv,
> }
>
>
> +/* Thread to query mdevctl for the current state of persistent mediated device
> + * defintions when any changes are detected */
> +static
> +void mdevctlThread(void *opaque G_GNUC_UNUSED)
> +{
> + udevEventDataPtr priv = driver->privateData;
> +
> + while (1) {
> + virObjectLock(priv);
> + while (!priv->mdevctlReady && !priv->threadQuit) {
> + if (virCondWait(&priv->mdevctlCond, &priv->parent.lock)) {
> + virReportSystemError(errno, "%s",
> + _("handler failed to wait on condition"));
> + virObjectUnlock(priv);
> + return;
> + }
> + }
> +
> + if (priv->threadQuit) {
> + virObjectUnlock(priv);
> + return;
> + }
> +
> + virObjectUnlock(priv);
> +
> + mdevctlEnumerateDevices();
> +
> + virObjectLock(priv);
> + priv->mdevctlReady = false;
> + virObjectUnlock(priv);
> + }
> +}
> +
> +
> /**
> * udevEventHandleThread
> * @opaque: unused
> @@ -1708,6 +1756,69 @@ udevEventHandleCallback(int watch G_GNUC_UNUSED,
> }
>
>
> +static int timeout_id;
> +
> +static gboolean
> +signalMdevctlThread(void *user_data)
> +{
> + udevEventDataPtr priv = user_data;
> +
> + if (timeout_id) {
> + g_source_remove(timeout_id);
> + timeout_id = 0;
> + }
> +
> + virObjectLock(priv);
> + priv->mdevctlReady = true;
> + virCondSignal(&priv->mdevctlCond);
> + virObjectUnlock(priv);
> +
> + return G_SOURCE_REMOVE;
> +}
> +
> +
> +static void
> +mdevctlEventHandleCallback(GFileMonitor *monitor G_GNUC_UNUSED,
> + GFile *file,
> + GFile *other_file G_GNUC_UNUSED,
> + GFileMonitorEvent event_type,
> + gpointer user_data)
> +{
> + udevEventDataPtr priv = user_data;
> +
> + /* if a new directory appears, monitor that directory for changes */
> + if (g_file_query_file_type(file, G_FILE_QUERY_INFO_NONE, NULL) ==
> + G_FILE_TYPE_DIRECTORY) {
> + GFileMonitor *mon;
> +
> + if ((mon = g_file_monitor(file, G_FILE_MONITOR_NONE, NULL, NULL))) {
> + g_signal_connect(mon, "changed",
> + G_CALLBACK(mdevctlEventHandleCallback),
> + user_data);
> + virObjectLock(priv);
> + priv->mdevctl_monitors = g_list_append(priv->mdevctl_monitors, mon);
> + virObjectUnlock(priv);
> + }
> + }
> +
> + /* Sometimes a single configuration change results in multiple notify
> + * events (e.g. several CHANGED events, or a CREATED and CHANGED event
Isn't this a glib bug that it sends multiple (even different) events for the
same change? Same for CHANGES_DONE_HINT, how come it may not arrive at the end
of all the changes?
Regards,
Erik
> + * followed by CHANGES_DONE_HINT). To avoid triggering the mdevctl thread
> + * multiple times for a single configuration change, try to coalesce these
> + * changes by waiting for the CHANGES_DONE_HINT event. As a fallback, add
> + * a timeout to trigger the signal if that event never comes */
> + if (event_type == G_FILE_MONITOR_EVENT_CREATED ||
> + event_type == G_FILE_MONITOR_EVENT_CHANGED) {
> + if (timeout_id)
> + g_source_remove(timeout_id);
> + timeout_id = g_timeout_add(100, signalMdevctlThread, priv);
> + return;
> + }
> +
> + signalMdevctlThread(priv);
> +}
> +
> +
> /* DMI is intel-compatible specific */
> #if defined(__x86_64__) || defined(__i386__) || defined(__amd64__)
> static void
> @@ -1858,6 +1969,58 @@ udevPCITranslateInit(bool privileged G_GNUC_UNUSED)
> }
>
>
> +/* Recursively monitors dir and any subdirectory for file changes and returns a
> + * GList of GFileMonitor objects */
> +static GList*
> +monitorDirRecursively(GFile *dir,
> + GCallback changed_cb,
> + gpointer user_data)
> +{
> + GList *monitors = NULL;
> + g_autoptr(GFileInfo) dirinfo = NULL;
> + g_autoptr(GError) error = NULL;
> + g_autoptr(GFileEnumerator) children = NULL;
> + GFileMonitor *mon;
> +
> + if (!(children = g_file_enumerate_children(dir, "standard::*",
> + G_FILE_QUERY_INFO_NONE, NULL, &error))) {
> + if (error->code == G_IO_ERROR_NOT_DIRECTORY)
> + return NULL;
> + goto bail;
> + }
> +
> + if (!(mon = g_file_monitor(dir, G_FILE_MONITOR_NONE, NULL, &error)))
> + goto bail;
> +
> + g_signal_connect(mon, "changed", changed_cb, user_data);
> + monitors = g_list_append(monitors, mon);
> +
> + while (true) {
> + GFileInfo *info;
> + GFile *child;
> + GList *child_monitors = NULL;
> +
> + if (!g_file_enumerator_iterate(children, &info, &child, NULL, &error))
> + goto bail;
> + if (!info)
> + break;
> +
> + child_monitors = monitorDirRecursively(child, changed_cb, user_data);
> + if (child_monitors)
> + monitors = g_list_concat(monitors, child_monitors);
> + }
> +
> + return monitors;
> +
> + bail:
> + if (error)
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to monitor directory: %s"), error->message);
> +
> + g_list_free_full(monitors, g_object_unref);
> + return NULL;
> +}
> +
> static int
> nodeStateInitialize(bool privileged,
> const char *root,
> @@ -1867,6 +2030,8 @@ nodeStateInitialize(bool privileged,
> udevEventDataPtr priv = NULL;
> struct udev *udev = NULL;
> virThread enumThread;
> + g_autoptr(GError) error = NULL;
> + g_autoptr(GFile) mdevctlConfigDir = g_file_new_for_path("/etc/mdevctl.d");
>
> if (root != NULL) {
> virReportError(VIR_ERR_INVALID_ARG, "%s",
> @@ -1969,6 +2134,23 @@ nodeStateInitialize(bool privileged,
> if (priv->watch == -1)
> goto unlock;
>
> + if (virThreadCreateFull(&priv->mdevctlThread, true, mdevctlThread,
> + "mdevctl-event", false, NULL) < 0) {
> + virReportSystemError(errno, "%s",
> + _("failed to create mdevctl handler thread"));
> + goto unlock;
> + }
> +
> + /* mdevctl may add notification events in the future:
> + * https://github.com/mdevctl/mdevctl/issues/27. For now, fall back to
> + * monitoring the mdevctl configuration directory for changes.
> + * mdevctl configuration is stored in a directory tree within
> + * /etc/mdevctl.d/. There is a directory for each parent device, which
> + * contains a file defining each mediated device */
> + priv->mdevctl_monitors = monitorDirRecursively(mdevctlConfigDir,
> + G_CALLBACK(mdevctlEventHandleCallback),
> + priv);
> +
> virObjectUnlock(priv);
>
> /* Create a fictional 'computer' device to root the device tree. */
> --
> 2.26.2
>
