[libvirt] [PATCH v5 1/2] util: Add netlink event handling to virnetlink.c
Laine Stump
laine at laine.org
Thu Feb 23 22:16:43 UTC 2012
On 02/22/2012 08:17 AM, D. Herrendoerfer wrote:
> From: "D. Herrendoerfer" <d.herrendoerfer at herrendoerfer.name>
>
> This code adds a netlink event interface to libvirt.
> It is based upon the event_poll code and makes use of
> it. An event is generated for each netlink message sent
> to the libvirt pid.
>
> Signed-off-by: D. Herrendoerfer <d.herrendoerfer at herrendoerfer.name>
> ---
> daemon/libvirtd.c | 8 +
> src/libvirt_private.syms | 6 +
> src/util/virnetlink.c | 438 +++++++++++++++++++++++++++++++++++++++++++++-
> src/util/virnetlink.h | 29 +++
> 4 files changed, 476 insertions(+), 5 deletions(-)
>
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index b1b542b..ca8074d 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -47,6 +47,7 @@
> #include "conf.h"
> #include "memory.h"
> #include "conf.h"
> +#include "virnetlink.h"
> #include "virnetserver.h"
> #include "threads.h"
> #include "remote.h"
> @@ -1598,6 +1599,12 @@ int main(int argc, char **argv) {
> goto cleanup;
> }
>
> + /* Register the netlink event service */
> + if (virNetlinkEventServiceStart() < 0) {
> + ret = VIR_DAEMON_ERR_NETWORK;
> + goto cleanup;
> + }
> +
> /* Run event loop. */
> virNetServerRun(srv);
>
> @@ -1607,6 +1614,7 @@ int main(int argc, char **argv) {
> 0, "shutdown", NULL);
>
> cleanup:
> + virNetlinkEventServiceStop();
> virNetServerProgramFree(remoteProgram);
> virNetServerProgramFree(qemuProgram);
> virNetServerClose(srv);
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index d6ad36c..008470e 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1258,6 +1258,12 @@ virNetDevVPortProfileOpTypeToString;
>
> #virnetlink.h
> virNetlinkCommand;
> +virNetlinkEventServiceIsRunning;
> +virNetlinkEventServiceStop;
> +virNetlinkEventServiceStart;
> +virNetlinkEventAddClient;
> +virNetlinkEventRemoveClient;
> +
>
>
> # virnetmessage.h
> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
> index d03d171..ec4727e 100644
> --- a/src/util/virnetlink.c
> +++ b/src/util/virnetlink.c
> @@ -35,7 +35,10 @@
> #include <sys/types.h>
>
> #include "virnetlink.h"
> +#include "logging.h"
> #include "memory.h"
> +#include "threads.h"
> +#include "virmacaddr.h"
> #include "virterror_internal.h"
>
> #define VIR_FROM_THIS VIR_FROM_NET
> @@ -46,6 +49,50 @@
>
> #define NETLINK_ACK_TIMEOUT_S 2
>
> +#if defined(__linux__) && defined(HAVE_LIBNL)
> +/* State for a single netlink event handle */
> +struct virNetlinkEventHandle {
> + int watch;
> + virNetlinkEventHandleCallback cb;
> + void *opaque;
> + unsigned char macaddr[VIR_MAC_BUFLEN];
> + int deleted;
> +};
> +
> +typedef struct _virNetlinkEventSrvPrivate virNetlinkEventSrvPrivate;
> +typedef virNetlinkEventSrvPrivate *virNetlinkEventSrvPrivatePtr;
> +struct _virNetlinkEventSrvPrivate {
> + /*Server*/
> + virMutex lock;
> + int eventwatch;
> + int netlinkfd;
> + struct nl_handle *netlinknh;
> + /*Events*/
> + int handled;
> + size_t handlesCount;
> + size_t handlesAlloc;
> + struct virNetlinkEventHandle *handles;
> +};
> +
> +enum virNetlinkDeleteMode {
> + VIR_NETLINK_HANDLE_VALID,
> + VIR_NETLINK_HANDLE_DELETED,
> +};
> +
> +/* Only have one event loop */
> +//static struct virNetlinkEventLoop eventLoop;
You commented this out but forgot to remove it.
> +
> +/* Unique ID for the next netlink watch to be registered */
> +static int nextWatch = 1;
> +
> +/* Allocate extra slots for virEventPollHandle/virEventPollTimeout
> + records in this multiple */
> +#define NETLINK_EVENT_ALLOC_EXTENT 10
> +
> +static virNetlinkEventSrvPrivatePtr server = NULL;
> +
> +/* Function definitions */
> +
> /**
> * virNetlinkCommand:
> * @nlmsg: pointer to netlink message
> @@ -58,7 +105,6 @@
> * Returns 0 on success, -1 on error. In case of error, no response
> * buffer will be returned.
> */
> -#if defined(__linux__) && defined(HAVE_LIBNL)
> int virNetlinkCommand(struct nl_msg *nl_msg,
> unsigned char **respbuf, unsigned int *respbuflen,
> int nl_pid)
> @@ -89,7 +135,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
> virReportSystemError(errno,
> "%s", _("cannot connect to netlink socket"));
> rc = -1;
> - goto err_exit;
> + goto error;
> }
>
> nlmsg_set_dst(nl_msg, &nladdr);
> @@ -101,7 +147,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
> virReportSystemError(errno,
> "%s", _("cannot send to netlink socket"));
> rc = -1;
> - goto err_exit;
> + goto error;
> }
>
> fd = nl_socket_get_fd(nlhandle);
> @@ -118,7 +164,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
> virReportSystemError(ETIMEDOUT, "%s",
> _("no valid netlink response was received"));
> rc = -1;
> - goto err_exit;
> + goto error;
> }
>
> *respbuflen = nl_recv(nlhandle, &nladdr, respbuf, NULL);
> @@ -127,7 +173,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
> "%s", _("nl_recv failed"));
> rc = -1;
> }
> -err_exit:
> +error:
> if (rc == -1) {
> VIR_FREE(*respbuf);
> *respbuf = NULL;
> @@ -138,6 +184,323 @@ err_exit:
> return rc;
> }
>
> +static void
> +virNetlinkEventServerLock(virNetlinkEventSrvPrivatePtr driver) {
> + virMutexLock(&driver->lock);
> +}
> +
> +static void
> +virNetlinkEventServerUnlock(virNetlinkEventSrvPrivatePtr driver) {
> + virMutexUnlock(&driver->lock);
> +}
> +
> +static void
> +virNetlinkEventCallback(int watch,
> + int fd ATTRIBUTE_UNUSED,
> + int events ATTRIBUTE_UNUSED,
> + void *opaque) {
> + virNetlinkEventSrvPrivatePtr srv = opaque;
> + unsigned char *msg;
> + struct sockaddr_nl peer;
> + struct ucred *creds = NULL;
> + int i, length;
> + bool handled = false;
> +
> + length = nl_recv(srv->netlinknh, &peer, &msg, &creds);
> +
> + if (length == 0)
> + return;
> + if (length < 0) {
> + netlinkError(errno,
> + "%s", _("nl_recv returned with error"));
> + return;
> + }
> +
> + virNetlinkEventServerLock(srv);
> +
> + VIR_DEBUG("dispatching to max %d clients, called from event watch %d",
> + (int)srv->handlesCount, watch);
> +
> + for (i = 0; i < srv->handlesCount; i++) {
> + if (srv->handles[i].deleted != VIR_NETLINK_HANDLE_VALID) {
> + continue;
> + }
> +
> + VIR_DEBUG("dispatching client %d.",i);
> +
> + virNetlinkEventHandleCallback cb = srv->handles[i].cb;
> + void *cpopaque = srv->handles[i].opaque;
> + (cb)( msg, length, &peer, &handled, cpopaque);
> + }
> +
> + if (!handled) {
> + VIR_DEBUG("nobody cared.");
We might want to say something a little less informal :-)
> + }
> +
> + VIR_FREE(msg);
> +
> + virNetlinkEventServerUnlock(srv);
> +}
> +
> +/**
> + * virNetlinkEventServiceStop:
> + *
> + * stop the monitor to receive netlink messages for libvirtd.
> + * This removes the netlink socket fd from the event handler.
> + *
> + * returns -1 if the monitor cannot be unregistered, 0 upon success
> + */
> +int
> +virNetlinkEventServiceStop(void) {
> + virNetlinkEventSrvPrivatePtr srv = server;
> +
> + VIR_INFO("stopping netlink event service");
> +
> + if (!server) {
> + return 0;
> + }
> +
> + virNetlinkEventServerLock(srv);
> +
> + nl_close(srv->netlinknh);
> + nl_handle_destroy(srv->netlinknh);
> +
> + virEventRemoveHandle(srv->eventwatch);
> + server=0;
> +
> + virNetlinkEventServerUnlock(srv);
> +
> + virMutexDestroy(&srv->lock);
> +
> + VIR_FREE(srv);
> +
> + return 0;
> +}
> +
> +/**
> + * virNetlinkEventServiceIsRunning:
> + *
> + * returns if the netlink event service is running.
> + *
> + * returns 'true' if the service is running, 'false' if stopped.
> + */
> +bool
> +virNetlinkEventServiceIsRunning(void) {
> + return (server != NULL);
> +}
> +
> +/**
> + * virNetlinkEventServiceStart:
> + *
> + * start a monitor to receive netlink messages for libvirtd.
> + * This registers a netlink socket with the event interface.
> + *
> + * returns -1 if the monitor cannot be registered, 0 upon success
> + */
> +int
> +virNetlinkEventServiceStart(void) {
> + virNetlinkEventSrvPrivatePtr srv;
> + int fd;
> + int ret = -1;
> +
> + if (server) {
> + return 0;
> + }
> +
> + VIR_INFO("starting netlink event service");
> +
> + if (VIR_ALLOC(srv) < 0) {
> + virReportOOMError();
> + goto error;
> + }
> +
> + /* Init the mutexes */
There's now only one mutex, and the name of the function makes the
comment a bit redundant.
> + if ( virMutexInit(&srv->lock) < 0)
> + goto error;
> +
> + virNetlinkEventServerLock(srv);
> +
> + /* Allocate a new socket and get fd */
> + srv->netlinknh = nl_handle_alloc();
> +
> + if (!srv->netlinknh) {
> + netlinkError(errno,
> + "%s", _("cannot allocate nlhandle for virNetlinkEvent server"));
> + goto error_locked;
> + }
> +
> + if (nl_connect(srv->netlinknh, NETLINK_ROUTE) < 0) {
> + netlinkError(errno,
> + "%s", _("cannot connect to netlink socket"));
> + goto error_server;
> + }
> +
> + fd = nl_socket_get_fd(srv->netlinknh);
> +
> + if (fd < 0) {
> + netlinkError(errno,
> + "%s", _("cannot get netlink socket fd"));
> + goto error_server;
> + }
> +
> + if (nl_socket_set_nonblocking(srv->netlinknh)) {
> + netlinkError(errno,
> + "%s", _("cannot set netlink socket nonblocking"));
> + goto error_server;
> + }
> +
> + if ((srv->eventwatch = virEventAddHandle(fd,
> + VIR_EVENT_HANDLE_READABLE,
> + virNetlinkEventCallback,
> + srv, NULL)) < 0) {
> + netlinkError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Failed to add netlink event handle watch"));
> +
> + goto error_server;
> + }
> +
> + srv->netlinkfd = fd;
> + VIR_DEBUG("netlink event listener on fd: %i running",fd);
> +
> + ret = 0;
> + server=srv;
> +
> +error_server:
> + if (ret == -1){
> + nl_close(srv->netlinknh);
> + nl_handle_destroy(srv->netlinknh);
> + }
> +error_locked:
> + virNetlinkEventServerUnlock(srv);
> + if (ret == -1) {
> + virMutexDestroy(&srv->lock);
> + VIR_FREE(srv);
> + }
> +error:
> + return ret;
> +}
> +
> +/**
> + * virNetlinkEventAddClient:
> + *
> + * @cb: callback to invoke when an event occurs
> + * @opaque: user data to pass to callback
> + * @macaddr: macaddr to store with the data. Used to identify callers. May be null.
> + *
> + * register a callback for handling of netlink messages. The
> + * registered function receives the entire netlink message and
> + * may choose to act upon it.
> + *
> + * returns -1 if the file handle cannot be registered, number of monitor upon success
> + */
> +int
> +virNetlinkEventAddClient(virNetlinkEventHandleCallback cb,
> + void *opaque,
> + const unsigned char *macaddr) {
> + int i,r, result;
> + virNetlinkEventSrvPrivatePtr srv = server;
> +
> + if ( cb == NULL )
> + return -1;
> +
> + virNetlinkEventServerLock(srv);
> +
> + VIR_DEBUG("adding client: %d.",nextWatch);
> +
> + r = 0;
> + /* first try to re-use deleted free slots */
> + for (i = 0; i < srv->handlesCount; i++) {
> + if (srv->handles[i].deleted == VIR_NETLINK_HANDLE_DELETED) {
> + r = i;
> + goto addentry;
> + }
> + }
> + /* Resize the eventLoop array if needed */
> + if (srv->handlesCount == srv->handlesAlloc) {
> + VIR_DEBUG("Used %zu handle slots, adding at least %d more",
> + srv->handlesAlloc, NETLINK_EVENT_ALLOC_EXTENT);
> + if (VIR_RESIZE_N(srv->handles, srv->handlesAlloc,
> + srv->handlesCount, NETLINK_EVENT_ALLOC_EXTENT) < 0) {
> + result = -1;
> + goto error;
> + }
> + }
> + r = srv->handlesCount++;
> +
> +addentry:
> + srv->handles[r].watch = nextWatch;
> + srv->handles[r].cb = cb;
> + srv->handles[r].opaque = opaque;
> + srv->handles[r].deleted = VIR_NETLINK_HANDLE_VALID;
> + if (macaddr)
> + memcpy(srv->handles[r].macaddr, macaddr, VIR_MAC_BUFLEN);
> +
> + VIR_DEBUG("added client to loop slot: %d. with macaddr ptr=%p", r, macaddr);
> +
> + result = nextWatch++;
> +
> +error:
> + virNetlinkEventServerUnlock(srv);
> +
> + return result;
> +}
> +
> +/**
> + * virNetlinkEventRemoveClient:
> + *
> + * @watch: watch whose handle to remove
> + * @macaddr: macaddr whose handle to remove
> + *
> + * Unregister a callback from a netlink monitor.
> + * The handler function referenced will no longer receive netlink messages.
> + * Either watch or macaddr may be used, the other should be null.
> + *
> + * returns -1 if the file handle was not registered, 0 upon success
> + */
> +int
> +virNetlinkEventRemoveClient(int watch, const unsigned char *macaddr) {
> + int i;
> + int ret = -1;
> + virNetlinkEventSrvPrivatePtr srv = server;
> +
> + VIR_DEBUG("removing client watch=%d, mac=%p.",
> + watch, macaddr);
> +
> + if (watch <= 0 && !macaddr) {
> + VIR_WARN("Ignoring invalid netlink client id: %d", watch);
> + return -1;
> + }
> +
> + virNetlinkEventServerLock(srv);
> +
> + for (i = 0; i < srv->handlesCount; i++) {
> + if (srv->handles[i].deleted != VIR_NETLINK_HANDLE_VALID)
> + continue;
> +
> + if (watch && srv->handles[i].watch == watch) {
> + VIR_FREE(srv->handles[i].opaque);
> + srv->handles[i].deleted = VIR_NETLINK_HANDLE_DELETED;
> + VIR_DEBUG("removed client: %d by index.",
> + srv->handles[i].watch);
> + ret = 0;
> + goto error;
> + }
> + if (!watch && memcmp(macaddr, srv->handles[i].macaddr, VIR_MAC_BUFLEN) == 0) {
> + VIR_FREE(srv->handles[i].opaque);
> + srv->handles[i].deleted = VIR_NETLINK_HANDLE_DELETED;
> + VIR_DEBUG("removed client: %d by mac.",
> + srv->handles[i].watch);
> + ret = 0;
> + goto error;
> + }
> + }
> + VIR_DEBUG("client not found to remove.");
> +
> +error:
> + virNetlinkEventServerUnlock(srv);
> + return ret;
> +}
> +
> #else
>
> int virNetlinkCommand(struct nl_msg *nl_msg ATTRIBUTE_UNUSED,
> @@ -154,4 +517,69 @@ int virNetlinkCommand(struct nl_msg *nl_msg ATTRIBUTE_UNUSED,
> return -1;
> }
>
> +/**
> + * stopNetlinkEventServer: stop the monitor to receive netlink messages for libvirtd
> + */
> +int virNetlinkEventServiceStop(void) {
> + netlinkError(VIR_ERR_INTERNAL_ERROR,
> + "%s",
> +# if defined(__linux__) && !defined(HAVE_LIBNL)
> + _("virNetlinkEventServiceStop is not supported since libnl was not available"));
> +# endif
> + return 0;
> +}
> +
> +/**
> + * startNetlinkEventServer: start a monitor to receive netlink messages for libvirtd
> + */
> +int virNetlinkEventServiceStart(void) {
> +# if defined(__linux__) && !defined(HAVE_LIBNL)
> + netlinkError(VIR_ERR_INTERNAL_ERROR,
> + "%s",
> + _("virNetlinkEventServiceStart is not supported since libnl was not available"));
> +# endif
> + return 0;
> +}
> +
> +/**
> + * virNetlinkEventServiceIsRunning: returns if the netlink event service is running.
> + */
> +int virNetlinkEventServiceIsRunning(void) {
> +# if defined(__linux__) && !defined(HAVE_LIBNL)
> + netlinkError(VIR_ERR_INTERNAL_ERROR,
> + "%s",
> + _("virNetlinkEventServiceIsRunning is not supported since libnl was not available"));
> +# endif
> + return 0;
> +}
> +
> +/**
> + * virNetlinkEventAddClient: register a callback for handling of netlink messages
> + */
> +int virNetlinkEventAddClient(virNetlinkEventHandleCallback cb, void *opaque,
> + const unsigned char *macaddr) {
> + netlinkError(VIR_ERR_INTERNAL_ERROR,
> + "%s",
> +# if defined(__linux__) && !defined(HAVE_LIBNL)
> + _("virNetlinkEventServiceAddClient is not supported since libnl was not available"));
> +# else
> + _("virNetlinkEventServiceAddClient is not supported on non-linux platforms"));
> +# endif
> + return -1;
> +}
> +
> +/**
> + * virNetlinkEventRemoveClient: unregister a callback from a netlink monitor
> + */
> +int virNetlinkEventRemoveClient(int watch, const unsigned char *macaddr) {
> + netlinkError(VIR_ERR_INTERNAL_ERROR,
> + "%s",
> +# if defined(__linux__) && !defined(HAVE_LIBNL)
> + _("virNetlinkEventRemoveClient is not supported since libnl was not available"));
> +# else
> + _("virNetlinkEventRemoveClient is not supported on non-linux platforms"));
> +# endif
> + return -1;
> +}
> +
> #endif /* __linux__ */
> diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
> index a70abb6..1365afa 100644
> --- a/src/util/virnetlink.h
> +++ b/src/util/virnetlink.h
> @@ -21,6 +21,7 @@
> # define __VIR_NETLINK_H__
>
> # include "config.h"
> +# include "internal.h"
>
> # if defined(__linux__) && defined(HAVE_LIBNL)
>
> @@ -29,6 +30,7 @@
> # else
>
> struct nl_msg;
> +struct sockaddr_nl;
>
> # endif /* __linux__ */
>
> @@ -36,4 +38,31 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
> unsigned char **respbuf, unsigned int *respbuflen,
> int nl_pid);
>
> +typedef void (*virNetlinkEventHandleCallback)( unsigned char *msg, int length, struct sockaddr_nl *peer, bool *handled, void *opaque);
> +
> +/**
> + * stopNetlinkEventServer: stop the monitor to receive netlink messages for libvirtd
> + */
> +int virNetlinkEventServiceStop(void);
> +
> +/**
> + * startNetlinkEventServer: start a monitor to receive netlink messages for libvirtd
> + */
> +int virNetlinkEventServiceStart(void);
> +
> +/**
> + * virNetlinkEventServiceIsRunning: returns if the netlink event service is running.
> + */
> +bool virNetlinkEventServiceIsRunning(void);
> +
> +/**
> + * virNetlinkEventAddClient: register a callback for handling of netlink messages
> + */
> +int virNetlinkEventAddClient(virNetlinkEventHandleCallback cb, void *opaque, const unsigned char *macaddr);
> +
> +/**
> + * virNetlinkEventRemoveClient: unregister a callback from a netlink monitor
> + */
> +int virNetlinkEventRemoveClient(int watch, const unsigned char *macaddr);
> +
> #endif /* __VIR_NETLINK_H__ */
This looks very good - just the three very minor nits. ACK to this; I'll
fix the nits and push it pending review of you answer to my question in
PATCH 2/2
More information about the libvir-list
mailing list