[libvirt] [PATCH 1/3] nodedev: Add driver callback mechanism to add/remove devices
Bjoern Walk
bwalk at linux.vnet.ibm.com
Thu Feb 9 12:52:41 UTC 2017
John Ferlan <jferlan at redhat.com> [2017-02-09, 04:36AM +0100]:
>Add a callback mechanism as a side door of sorts to the event
>mgmt functions that are triggered when a node_device object is
>added or removed. This includes a mechanism to enumerate already
>added devices for those stateInitialize functions called after
>the initial nodedevRegister is called.
>
>Signed-off-by: John Ferlan <jferlan at redhat.com>
>---
> src/check-aclrules.pl | 3 +-
> src/check-driverimpls.pl | 1 +
> src/conf/node_device_conf.c | 125 ++++++++++++++++++++++++++++++++++++-
> src/conf/node_device_conf.h | 18 ++++++
> src/libvirt_private.syms | 3 +
> src/node_device/node_device_udev.c | 29 +++++++++
> 6 files changed, 177 insertions(+), 2 deletions(-)
>
>diff --git a/src/check-aclrules.pl b/src/check-aclrules.pl
>index 8739cda..161d954 100755
>--- a/src/check-aclrules.pl
>+++ b/src/check-aclrules.pl
>@@ -243,7 +243,8 @@ while (<>) {
> } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+/) {
> if ($1 ne "virNWFilterCallbackDriver" &&
> $1 ne "virNWFilterTechDriver" &&
>- $1 ne "virDomainConfNWFilterDriver") {
>+ $1 ne "virDomainConfNWFilterDriver" &&
>+ $1 ne "virNodeDeviceCallbackDriver") {
> $intable = 1;
> $table = $1;
> }
>diff --git a/src/check-driverimpls.pl b/src/check-driverimpls.pl
>index e320558..b707fc8 100755
>--- a/src/check-driverimpls.pl
>+++ b/src/check-driverimpls.pl
>@@ -70,6 +70,7 @@ while (<>) {
> } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+/) {
> next if $1 eq "virNWFilterCallbackDriver" ||
> $1 eq "virNWFilterTechDriver" ||
>+ $1 eq "virNodeDeviceCallbackDriver" ||
> $1 eq "virConnectDriver";
> $intable = 1;
> $table = $1;
>diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
>index 4d3268f..7a4df44 100644
>--- a/src/conf/node_device_conf.c
>+++ b/src/conf/node_device_conf.c
>@@ -33,6 +33,7 @@
> #include "virstring.h"
> #include "node_device_conf.h"
> #include "device_conf.h"
>+#include "virlog.h"
> #include "virxml.h"
> #include "virbuffer.h"
> #include "viruuid.h"
>@@ -40,6 +41,8 @@
>
> #define VIR_FROM_THIS VIR_FROM_NODEDEV
>
>+VIR_LOG_INIT("conf.node_device_conf");
>+
> VIR_ENUM_IMPL(virNodeDevCap, VIR_NODE_DEV_CAP_LAST,
> "system",
> "pci",
>@@ -263,6 +266,104 @@ void virNodeDeviceDefFree(virNodeDeviceDefPtr def)
> VIR_FREE(def);
> }
>
>+
>+/* Provide callback infrastructure that is expected to be called when
>+ * devices are added/removed from a node_device subsystem that supports
>+ * the callback mechanism. This provides a way for drivers to register
>+ * to be notified when a node_device is added/removed. */
>+virNodedevEnumerateAddDevices virNodedevEnumerateAddDevicesCb = NULL;
>+int nodeDeviceCallbackDriver;
>+#define MAX_CALLBACK_DRIVER 10
>+static virNodeDeviceCallbackDriverPtr nodeDeviceDrvArray[MAX_CALLBACK_DRIVER];
>+
>+
>+/**
>+ * @cb: Driver function to be called.
>+ *
>+ * Set a callback at driver initialization to provide a callback to a
>+ * driver specific function that can handle the enumeration of the existing
>+ * devices and the addition of those devices for the registering driver.
>+ *
>+ * The initial node_device enumeration is done prior to other drivers, thus
>+ * this provides a mechanism to load the existing data since the functions
>+ * virNodeDeviceAssignDef and virNodeDeviceObjRemove would typically
>+ * only be called when a new device is added/removed after the initial
>+ * enumeration. The registering driver will need to handle duplication
>+ * of data.
>+ */
>+void
>+virNodeDeviceConfEnumerateInit(virNodedevEnumerateAddDevices cb)
>+{
>+ virNodedevEnumerateAddDevicesCb = cb;
>+}
>+
>+
>+/**
>+ * @cbd: Driver callback pointers to add/remove devices
>+ *
>+ * Register a callback function in the registering driver to be called
>+ * when devices are added or removed. Additionally, ensure the initial
>+ * enumeration of the devices is completed taking care to do it after
>+ * setting the callbacks
>+ *
>+ * Returns 0 on success, -1 on failure
>+ */
>+int
>+virNodeDeviceRegisterCallbackDriver(virNodeDeviceCallbackDriverPtr cbd)
>+{
>+ if (nodeDeviceCallbackDriver >= MAX_CALLBACK_DRIVER) {
>+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>+ _("no space available for another callback driver"));
>+ return -1;
>+ }
>+
>+ if (!cbd->nodeDeviceAdd || !cbd->nodeDeviceRemove) {
>+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>+ _("callback requires both add and remove functions"));
>+ return -1;
>+ }
>+
>+ nodeDeviceDrvArray[nodeDeviceCallbackDriver++] = cbd;
>+
>+ /* Setting the add/remove callback first ensures that there is no
>+ * window of opportunity for a device to be added after enumeration
>+ * is complete, but before the callback is in place. So, set the
>+ * callback first, then do the enumeration. */
>+ if (virNodedevEnumerateAddDevicesCb) {
>+ if (virNodedevEnumerateAddDevicesCb(cbd->nodeDeviceAdd) < 0) {
>+ nodeDeviceDrvArray[--nodeDeviceCallbackDriver] = NULL;
>+ return -1;
>+ }
>+ }
>+
>+ return 0;
>+}
>+
>+
>+/**
>+ * @cb: Driver function to be called.
>+ *
>+ * Clear the callback function. It'll be up to the calling driver to
>+ * clear it's own data properly
>+ */
>+void
>+virNodeDeviceUnregisterCallbackDriver(virNodeDeviceCallbackDriverPtr cbd)
>+{
>+ size_t i = 0;
>+
>+ while (i < nodeDeviceCallbackDriver && nodeDeviceDrvArray[i] != cbd)
>+ i++;
>+
>+ if (i < nodeDeviceCallbackDriver) {
>+ memmove(&nodeDeviceDrvArray[i], &nodeDeviceDrvArray[i+1],
>+ (nodeDeviceCallbackDriver - i - 1) *
>+ sizeof(nodeDeviceDrvArray[i]));
>+ nodeDeviceDrvArray[i] = NULL;
>+ nodeDeviceCallbackDriver--;
>+ }
>+}
>+
>+
> void virNodeDeviceObjFree(virNodeDeviceObjPtr dev)
> {
> if (!dev)
>@@ -289,6 +390,7 @@ void virNodeDeviceObjListFree(virNodeDeviceObjListPtr devs)
> virNodeDeviceObjPtr virNodeDeviceAssignDef(virNodeDeviceObjListPtr devs,
> virNodeDeviceDefPtr def)
> {
>+ size_t i;
> virNodeDeviceObjPtr device;
>
> if ((device = virNodeDeviceFindByName(devs, def->name))) {
>@@ -315,6 +417,18 @@ virNodeDeviceObjPtr virNodeDeviceAssignDef(virNodeDeviceObjListPtr devs,
> }
> device->def = def;
>
>+ /* Call any registered drivers that want to be notified of a new device */
>+ for (i = 0; i < nodeDeviceCallbackDriver; i++) {
>+ if (nodeDeviceDrvArray[i]->nodeDeviceAdd(def, false) < 0) {
>+ VIR_DELETE_ELEMENT(devs->objs, devs->count - 1, devs->count);
I don't understand the reasoning why a failure to process the device on
one listening driver leads to the complete rejection of the device in
the node device driver.
>+ virNodeDeviceObjUnlock(device);
>+ virNodeDeviceObjFree(device);
>+ /* NB: If we fail to remove from one driver - it's not a problem
>+ * since adding a new device wouldn't fail if already found */
>+ return NULL;
>+ }
>+ }
>+
> return device;
>
> }
>@@ -322,13 +436,22 @@ virNodeDeviceObjPtr virNodeDeviceAssignDef(virNodeDeviceObjListPtr devs,
> void virNodeDeviceObjRemove(virNodeDeviceObjListPtr devs,
> virNodeDeviceObjPtr *dev)
> {
>- size_t i;
>+ size_t i, j;
>
> virNodeDeviceObjUnlock(*dev);
>
> for (i = 0; i < devs->count; i++) {
> virNodeDeviceObjLock(*dev);
> if (devs->objs[i] == *dev) {
>+ /* Call any registered drivers that want to be notified of a
>+ * removed device */
>+ for (j = 0; j < nodeDeviceCallbackDriver; j++) {
>+ if (nodeDeviceDrvArray[j]->nodeDeviceRemove((*dev)->def) < 0) {
>+ VIR_DEBUG("failed to remove name='%s' parent='%s' from "
>+ "callback driver, continuing",
>+ (*dev)->def->name, (*dev)->def->parent);
>+ }
>+ }
> virNodeDeviceObjUnlock(*dev);
> virNodeDeviceObjFree(devs->objs[i]);
> *dev = NULL;
>diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
>index 1634483..925cb6e 100644
>--- a/src/conf/node_device_conf.h
>+++ b/src/conf/node_device_conf.h
>@@ -207,6 +207,24 @@ struct _virNodeDeviceDef {
> virNodeDevCapsDefPtr caps; /* optional device capabilities */
> };
>
>+/* Callback mechanism to add/remove node device's by name/parent
>+ * to a target driver that cares to know */
>+typedef int (*virNodeDeviceAdd)(virNodeDeviceDefPtr def, bool enumerate);
>+typedef int (*virNodeDeviceRemove)(virNodeDeviceDefPtr def);
>+
>+typedef struct _virNodeDeviceCallbackDriver virNodeDeviceCallbackDriver;
>+typedef virNodeDeviceCallbackDriver *virNodeDeviceCallbackDriverPtr;
>+struct _virNodeDeviceCallbackDriver {
>+ const char *name;
>+ virNodeDeviceAdd nodeDeviceAdd;
>+ virNodeDeviceRemove nodeDeviceRemove;
>+};
>+
>+typedef int (*virNodedevEnumerateAddDevices)(virNodeDeviceAdd deviceAddCb);
>+void virNodeDeviceConfEnumerateInit(virNodedevEnumerateAddDevices cb);
>+
>+int virNodeDeviceRegisterCallbackDriver(virNodeDeviceCallbackDriverPtr);
>+void virNodeDeviceUnregisterCallbackDriver(virNodeDeviceCallbackDriverPtr);
>
> typedef struct _virNodeDeviceObj virNodeDeviceObj;
> typedef virNodeDeviceObj *virNodeDeviceObjPtr;
>diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>index d556c7d..6d11463 100644
>--- a/src/libvirt_private.syms
>+++ b/src/libvirt_private.syms
>@@ -695,6 +695,7 @@ virNodeDevCapsDefFree;
> virNodeDevCapTypeFromString;
> virNodeDevCapTypeToString;
> virNodeDeviceAssignDef;
>+virNodeDeviceConfEnumerateInit;
> virNodeDeviceDefFormat;
> virNodeDeviceDefFree;
> virNodeDeviceDefParseFile;
>@@ -713,6 +714,8 @@ virNodeDeviceObjListFree;
> virNodeDeviceObjLock;
> virNodeDeviceObjRemove;
> virNodeDeviceObjUnlock;
>+virNodeDeviceRegisterCallbackDriver;
>+virNodeDeviceUnregisterCallbackDriver;
>
>
> # conf/node_device_event.h
>diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
>index 4b81312..ea6970b 100644
>--- a/src/node_device/node_device_udev.c
>+++ b/src/node_device/node_device_udev.c
>@@ -1457,6 +1457,33 @@ static int udevPCITranslateInit(bool privileged ATTRIBUTE_UNUSED)
> return 0;
> }
>
>+
>+/*
>+ * @deviceAddCb: Callback routine for adding a device
>+ *
>+ * Enumerate all known devices calling the add device callback function
>+ *
>+ * Returns 0 on success, -1 on failure
>+ */
>+static int
>+udevEnumerateAddDevices(virNodeDeviceAdd deviceAddCb)
>+{
>+ size_t i;
>+ int ret = 0;
>+
>+ nodeDeviceLock();
>+ for (i = 0; i < driver->devs.count && ret >= 0; i++) {
>+ virNodeDeviceObjPtr obj = driver->devs.objs[i];
>+ virNodeDeviceObjLock(obj);
>+ ret = deviceAddCb(obj->def, true);
>+ virNodeDeviceObjUnlock(obj);
>+ }
>+ nodeDeviceUnlock();
>+
>+ return ret;
>+}
>+
>+
> static int nodeStateInitialize(bool privileged,
> virStateInhibitCallback callback ATTRIBUTE_UNUSED,
> void *opaque ATTRIBUTE_UNUSED)
>@@ -1535,6 +1562,8 @@ static int nodeStateInitialize(bool privileged,
> if (udevEnumerateDevices(udev) != 0)
> goto cleanup;
>
>+ virNodeDeviceConfEnumerateInit(udevEnumerateAddDevices);
I don't quite see the need for this callback indirection. What other
drivers want to implement a different enumeration method for devices?
>+
> ret = 0;
>
> cleanup:
>--
>2.7.4
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
>
--
IBM Systems
Linux on z Systems & Virtualization Development
------------------------------------------------------------------------
IBM Deutschland
Schönaicher Str. 220
71032 Böblingen
Phone: +49 7031 16 1819
E-Mail: bwalk at de.ibm.com
------------------------------------------------------------------------
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 896 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170209/b45cd9b3/attachment-0001.sig>
More information about the libvir-list
mailing list