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

Thursday, 23 February 2012

On 02/22/2012 08:17 AM, D. Herrendoerfer wrote:
...
 From: "D. Herrendoerfer"
<d.herrendoerfer(a)herrendoerfer.name&gt;

 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(a)herrendoerfer.name&gt;
 ---
  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

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

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