[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v5 1/2] util: Add netlink event handling to virnetlink.c



On 02/22/2012 08:17 AM, D. Herrendoerfer wrote:
> From: "D. Herrendoerfer" <d herrendoerfer 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 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]