[libvirt] [libvirt-snmp][PATCH] Add SNMP trap/notification support.
Laine Stump
laine at laine.org
Thu Feb 24 18:16:25 UTC 2011
(I'm unfamiliar not only with the libvirt-snmp code, but also mib2c, so
I can't claim any special qualification to review this. I'll just stick
to the basics...) (ie full grammar nazi mode engaged :-)
This patch doesn't apply cleanly - it complains of trailing whitespace,
and the patch to INSTALL doesn't apply at all. I finally ran the
patchfile through dos2unix, applied it with patch, then made the changes
to INSTALL by hand (because it still wouldn't apply).
There was also an extra blank at the end of a line (noted below) that
will prevent this from being pushed to origin/master. It's in a
generated file, but you say that file was tweaked by hand, so this extra
tweak won't create too much of a problem.
The most important comment I made below is that a large amount of this
patch seems to be whitespace changes (especially in libvirtSnmp.c and
libvirtSnmp.h). Both from a review and a maintenance point of view, I
think it would be a really good idea to separate the whitespace changes
and the functional changes into two separate patches.
On 02/16/2011 09:19 AM, Michal Privoznik wrote:
> This patch adds support for domain lifecycle notification support
> over SNMP traps. SNMP subagent monitors any domain events and when
> something interesting happens, it sends trap.
"sends *a* trap"
> Monitoring is done in a joinable thread using polling (used
> domain-events example from libvirt) so we won't block agent itself.
won't block *the* agent itself.
> Some debug info can be printed out by setting VERBOSE environment
> variable.
Is this a convention for SNMP agents? If not, would a more specific name
be more appropriate?
> ---
>
> Finally, we have a proof-of-concept implementation of SNMP trap.
> Two new files were first generated using mib2c then edited, so be
> careful when re-generating them.
>
> Destination of traps is defined in snmpd.conf (trap2sink), and to
> recieve them snmptrapd must be configured an up.
>
> The idea behind is - create a thread in which we poll for domain
> events. Once we receive event notification, we just fire up
> trap sending.
>
> I know it is a quite big patch, but nearly a half of it is just
> domain event registration.
> INSTALL | 4 +-
> configure.ac | 7 +
> src/LIBVIRT-MIB.txt | 9 +
> src/Makefile.am | 9 +-
> src/README.txt | 6 +
> src/libvirtNotifications.c | 105 +++++++++
> src/libvirtNotifications.h | 16 ++
> src/libvirtSnmp.c | 541 +++++++++++++++++++++++++++++++++-----------
> src/libvirtSnmp.h | 15 +-
> 9 files changed, 571 insertions(+), 141 deletions(-)
> create mode 100644 src/libvirtNotifications.c
> create mode 100644 src/libvirtNotifications.h
>
> diff --git a/INSTALL b/INSTALL
> index 31345d8..5c26d1e 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -18,10 +18,12 @@ Now it's time for make:
> This compile all sources producing runable SNMP subagent
> libvirtMib_subagent, which is installed right after.
"This compiles all source producing a runnable SNMP subagent,
libvirtMib_subagent, which is installed afterward."
> But before we run it, we need to edit /etc/snmp/snmpd.conf
> -so it contains this two lines:
>
> +so it contains this four lines:
"so it contains these four lines:"
>
> rwcommunity public
> master agentx
> +trap2sink localhost
> +trapcommunity public
>
> and then restart snmpd:
> /etc/init.d/snmpd restart
You might want to add a blank line between those two while you're
touching the file.
> diff --git a/configure.ac b/configure.ac
> index a5fcb4e..58d26b2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -86,5 +86,12 @@ fi
>
> AC_SUBST([MIB_DIR])
>
> +dnl pthread
> +PTHREAD_LIBS=
> +AC_CHECK_HEADERS(pthread.h, [], [AC_MSG_ERROR([pthread.h required])])
> +AC_CHECK_LIB(pthread, pthread_create, [PTHREAD_LIBS="-lpthread"])
> +
> +AC_SUBST([PTHREAD_LIBS])
> +
> AC_OUTPUT(Makefile src/Makefile libvirt-snmp.spec)
>
> diff --git a/src/LIBVIRT-MIB.txt b/src/LIBVIRT-MIB.txt
> index 607d441..932dec9 100644
> --- a/src/LIBVIRT-MIB.txt
> +++ b/src/LIBVIRT-MIB.txt
> @@ -201,5 +201,14 @@ libvirtGuestGroup OBJECT-GROUP
> guests."
> ::= { libvirtGroups 1 }
>
> +libvirtGuestNotif NOTIFICATION-TYPE
> + STATUS current
> + OBJECTS { libvirtGuestName,
> + libvirtGuestUUID,
> + libvirtGuestState,
> + libvirtGuestRowStatus }
> + DESCRIPTION
> + "Guest lifecycle notification."
> + ::= { libvirtNotifications 1 }
>
> END
> diff --git a/src/Makefile.am b/src/Makefile.am
> index c781e23..98fe562 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -9,19 +9,22 @@ AM_CFLAGS = \
>
> AM_LDFLAGS = \
> $(COVERAGE_LDFLAGS) \
> - $(SNMP_LIBS)
> + $(SNMP_LIBS) \
> + $(PTHREAD_LIBS)
>
> USER_SRCS = \
> libvirtGuestTable_data_get.c \
> libvirtGuestTable_data_set.c \
> libvirtGuestTable_data_access.c \
> - libvirtSnmp.c
> + libvirtSnmp.c \
> + libvirtNotifications.c
>
> USER_HDRS = \
> libvirtGuestTable_data_get.h \
> libvirtGuestTable_data_set.h \
> libvirtGuestTable_data_access.h \
> - libvirtSnmp.h
> + libvirtSnmp.h \
> + libvirtNotifications.h
>
> SRCS = \
> ${USER_SRCS} \
> diff --git a/src/README.txt b/src/README.txt
> index 22dd2af..9334a14 100644
> --- a/src/README.txt
> +++ b/src/README.txt
I notice that the README.txt file doesn't insert "courtesy newlines"
into paragraphs, instead each paragraph is an extremely long single
line. That's not necessarily related to this patch, but you might want
to format it so it's easier to read with more, emacs, etc.
> @@ -34,6 +34,10 @@ libvirtGuestTable*
> - I've done everything necessary for simple read-write access.
> - This code must be regenerated when the definition of libvirtGuestTable in the MIB file is changed! There is some automatic merging tool, which merges my changes with newly generated code, but I wouldn't rely on it.
>
> +libvirtNotifications.[c|h]
> +- The SNMP trap (notification) stuff
> +- Generated by this command: 'MIBDIRS="+." MIBS="+LIBVIRT-MIB" mib2c -c mib2c.notify.conf libvirtNotifications'
> +- and slightly modified (especially filling up trap variables which are going to be send).
s/send/sent/
>
> Usage (tested on Fedora 14 and RHEL6)
> -------------------------------------
> @@ -44,6 +48,8 @@ $ make
> 2. use following /etc/snmp/snmpd.conf:
> rwcommunity public
> master agentx
> +trap2sink localhost
> +trapcommunity public
I would insert a blank line before the example snmpd.conf, and indent
each line a couple spaces so that it's obvious what goes in the file.
Likewise for the other points in this list. That can be done in a
separate patch that just reformats the document without changing any
content, though.
>
> 3. service snmpd start
>
> diff --git a/src/libvirtNotifications.c b/src/libvirtNotifications.c
> new file mode 100644
> index 0000000..90402a6
> --- /dev/null
> +++ b/src/libvirtNotifications.c
> @@ -0,0 +1,105 @@
> +
> +/*
> + * Note: this file originally auto-generated by mib2c using
> + * : mib2c.notify.conf 17455 2009-04-05 09:53:29Z magfr $
> + */
1) as long as this file is being modified, does it rate getting a
Copyright notice?
2) It would be helpful to those of us who are unfamiliar with all this
if the places you had modified after the auto-generation were clearly
marked. (a lot of it is obvious, ie anything with "vir" or "libvirt",
but being a noob to mib2c, I'm not sure about the rest).
> +
> +#include<net-snmp/net-snmp-config.h>
> +#include<net-snmp/net-snmp-includes.h>
> +#include<net-snmp/agent/net-snmp-agent-includes.h>
> +#include<string.h>
> +#include "libvirtNotifications.h"
> +#include "libvirtGuestTable_enums.h"
> +
> +static const oid snmptrap_oid[] = { 1, 3, 6, 1, 6, 3, 1, 1, 4, 1, 0 };
> +
> +int
> +send_libvirtGuestNotif_trap(virDomainPtr dom)
> +{
> + netsnmp_variable_list *var_list = NULL;
> + const oid libvirtGuestNotif_oid[] = { 1, 3, 6, 1, 4, 1, 12345, 0, 1 };
> + const oid libvirtGuestName_oid[] =
> + { 1, 3, 6, 1, 4, 1, 12345, 1, 1, 1, 2, 0 };
> + const oid libvirtGuestUUID_oid[] =
> + { 1, 3, 6, 1, 4, 1, 12345, 1, 1, 1, 1, 1 };
> + const oid libvirtGuestState_oid[] =
> + { 1, 3, 6, 1, 4, 1, 12345, 1, 1, 1, 3, 2 };
> + const oid libvirtGuestRowStatus_oid[] =
> + { 1, 3, 6, 1, 4, 1, 12345, 1, 1, 1, 9, 3 };
> +
> +
> + const char *domName = virDomainGetName(dom);
> + char domUUID[VIR_UUID_STRING_BUFLEN];
> + virDomainInfo info;
> + int rowstatus = ROWSTATUS_ACTIVE;
> +
> + if (virDomainGetUUIDString(dom, domUUID)) {
> + fprintf(stderr, "Failed to get domain UUID\n");
> + return SNMP_ERR_GENERR;
> + }
> +
> + if (virDomainGetInfo(dom,&info)) {
> + fprintf(stderr, "Failed to get domain info\n");
> + return SNMP_ERR_GENERR;
> + }
> +
> + /*
> + * If domain is shuting down, row in libvirtGuestTable will
s/shuting/shutting/
> + * not be accessible anymore.
> + */
> + switch (info.state) {
> + case VIR_DOMAIN_SHUTDOWN:
> + case VIR_DOMAIN_SHUTOFF:
> + case VIR_DOMAIN_CRASHED:
> + rowstatus = ROWSTATUS_NOTINSERVICE;
> + break;
> +
> + default:
> + rowstatus = ROWSTATUS_ACTIVE;
> + break;
> + };
> +
> + /*
> + * Set the snmpTrapOid.0 value
> + */
> + snmp_varlist_add_variable(&var_list,
> + snmptrap_oid, OID_LENGTH(snmptrap_oid),
> + ASN_OBJECT_ID,
> + libvirtGuestNotif_oid,
> + sizeof(libvirtGuestNotif_oid));
> +
> + /*
> + * Add any objects from the trap definition
> + */
> + snmp_varlist_add_variable(&var_list,
> + libvirtGuestName_oid,
> + OID_LENGTH(libvirtGuestName_oid),
> + ASN_OCTET_STR, domName, strlen(domName));
> + snmp_varlist_add_variable(&var_list,
> + libvirtGuestUUID_oid,
> + OID_LENGTH(libvirtGuestUUID_oid),
> + ASN_OCTET_STR, domUUID, sizeof(domUUID));
> + snmp_varlist_add_variable(&var_list,
> + libvirtGuestState_oid,
> + OID_LENGTH(libvirtGuestState_oid),
> + ASN_INTEGER,
> + (u_char *)& rowstatus, sizeof(rowstatus));
> + snmp_varlist_add_variable(&var_list,
> + libvirtGuestRowStatus_oid,
> + OID_LENGTH(libvirtGuestRowStatus_oid),
> + ASN_INTEGER,
> + (u_char *)& info.state, sizeof(info.state));
> +
> + /*
> + * Add any extra (optional) objects here
> + */
> +
> + /*
> + * Send the trap to the list of configured destinations
> + * and clean up
> + */
> + send_v2trap(var_list);
> + snmp_free_varbind(var_list);
> +
> + return SNMP_ERR_NOERROR;
> +}
> diff --git a/src/libvirtNotifications.h b/src/libvirtNotifications.h
> new file mode 100644
> index 0000000..aed4560
> --- /dev/null
> +++ b/src/libvirtNotifications.h
> @@ -0,0 +1,16 @@
> +
> +/*
> + * Note: this file originally auto-generated by mib2c using
> + * : mib2c.notify.conf 17455 2009-04-05 09:53:29Z magfr $
> + */
> +#ifndef LIBVIRTNOTIFICATIONS_H
> +#define LIBVIRTNOTIFICATIONS_H
> +
> +#include "libvirtSnmp.h"
> +
> +/*
> + * function declarations
The line above is the one with a space at the end, preventing "git am"
from working on the patch (I think it will also prevent you from pushing
it to origin/master).
> + */
> +int send_libvirtGuestNotif_trap(virDomainPtr dom);
> +
> +#endif /* LIBVIRTNOTIFICATIONS_H */
> diff --git a/src/libvirtSnmp.c b/src/libvirtSnmp.c
> index c15431a..4698f89 100644
> --- a/src/libvirtSnmp.c
> +++ b/src/libvirtSnmp.c
> @@ -1,3 +1,4 @@
> +
> /* This file contains trivial example code to connect to the running
> * hypervisor and gather a few bits of information. */
>
> @@ -9,12 +10,171 @@ gcc -lvirt activeguests.c
>
> #include<stdio.h>
> #include<stdlib.h>
> +#include<sys/types.h>
> +#include<sys/poll.h>
> +#include<pthread.h>
> +#include<signal.h>
>
> #include "libvirtSnmp.h"
> +
> /* include our MIB structures*/
> #include "libvirtGuestTable.h"
> +#include "libvirtNotifications.h"
> +
> +#define DEBUG0(fmt) if (verbose) printf("%s:%d :: " fmt "\n", \
> + __func__, __LINE__)
> +#define DEBUG(fmt, ...) if (verbose) printf("%s:%d: " fmt "\n", \
> + __func__, __LINE__, __VA_ARGS__)
> +#define STREQ(a,b) (strcmp(a,b) == 0)
>
> +#ifndef ATTRIBUTE_UNUSED
> +#define ATTRIBUTE_UNUSED __attribute__((__unused__))
> +#endif
> +
> +int verbose = 0;
> virConnectPtr conn;
> +int callbackRet = -1;
> +int run = 1;
> +pthread_t poll_thread;
> +
> +/* handle globals */
> +int h_fd = 0;
> +virEventHandleType h_event = 0;
> +virEventHandleCallback h_cb = NULL;
> +virFreeCallback h_ff = NULL;
> +void *h_opaque = NULL;
> +
> +/* timeout globals */
> +#define TIMEOUT_MS 1000
> +int t_active = 0;
> +int t_timeout = -1;
> +virEventTimeoutCallback t_cb = NULL;
> +virFreeCallback t_ff = NULL;
> +void *t_opaque = NULL;
> +
> +
> +static int
> +domainEventCallback(virConnectPtr conn ATTRIBUTE_UNUSED,
> + virDomainPtr dom, int event, int detail,
> + void *opaque ATTRIBUTE_UNUSED)
> +{
> + DEBUG("%s EVENT: Domain %s(%d) %d %d\n", __func__,
> + virDomainGetName(dom), virDomainGetID(dom), event, detail);
> +
> + send_libvirtGuestNotif_trap(dom);
> + return 0;
> +}
> +
> +static void
> +myFreeFunc(void *opaque)
> +{
> + if (opaque)
> + free(opaque);
> +}
> +
> +/* EventImpl Functions */
> +int
> +myEventHandleTypeToPollEvent(virEventHandleType events)
> +{
> + int ret = 0;
> +
> + if (events& VIR_EVENT_HANDLE_READABLE)
> + ret |= POLLIN;
> + if (events& VIR_EVENT_HANDLE_WRITABLE)
> + ret |= POLLOUT;
> + if (events& VIR_EVENT_HANDLE_ERROR)
> + ret |= POLLERR;
> + if (events& VIR_EVENT_HANDLE_HANGUP)
> + ret |= POLLHUP;
> + return ret;
> +}
> +
> +virEventHandleType
> +myPollEventToEventHandleType(int events)
> +{
> + virEventHandleType ret = 0;
> +
> + if (events& POLLIN)
> + ret |= VIR_EVENT_HANDLE_READABLE;
> + if (events& POLLOUT)
> + ret |= VIR_EVENT_HANDLE_WRITABLE;
> + if (events& POLLERR)
> + ret |= VIR_EVENT_HANDLE_ERROR;
> + if (events& POLLHUP)
> + ret |= VIR_EVENT_HANDLE_HANGUP;
> + return ret;
> +}
> +
> +int
> +myEventAddHandleFunc(int fd, int event,
> + virEventHandleCallback cb,
> + void *opaque, virFreeCallback ff)
> +{
> + DEBUG("Add handle %d %d %p %p", fd, event, cb, opaque);
> + h_fd = fd;
> + h_event = myEventHandleTypeToPollEvent(event);
> + h_cb = cb;
> + h_ff = ff;
> + h_opaque = opaque;
> + return 0;
> +}
> +
> +void
> +myEventUpdateHandleFunc(int fd, int event)
> +{
> + DEBUG("Updated Handle %d %d", fd, event);
> + h_event = myEventHandleTypeToPollEvent(event);
> + return;
> +}
> +
> +int
> +myEventRemoveHandleFunc(int fd)
> +{
> + DEBUG("Removed Handle %d", fd);
> + h_fd = 0;
> + if (h_ff)
> + (h_ff) (h_opaque);
> + return 0;
> +}
> +
> +int
> +myEventAddTimeoutFunc(int timeout,
> + virEventTimeoutCallback cb,
> + void *opaque, virFreeCallback ff)
> +{
> + DEBUG("Adding Timeout %d %p %p", timeout, cb, opaque);
> + t_active = 1;
> + t_timeout = timeout;
> + t_cb = cb;
> + t_ff = ff;
> + t_opaque = opaque;
> + return 0;
> +}
> +
> +void
> +myEventUpdateTimeoutFunc(int timer ATTRIBUTE_UNUSED, int timeout)
> +{
> + /*DEBUG("Timeout updated %d %d", timer, timeout); */
> + t_timeout = timeout;
> +}
> +
> +int
> +myEventRemoveTimeoutFunc(int timer)
> +{
> + DEBUG("Timeout removed %d", timer);
> + t_active = 0;
> + if (t_ff)
> + (t_ff) (t_opaque);
> + return 0;
> +}
> +
> +/* Signal trap function */
> +static void
> +stop(int sig)
> +{
> + run = 0;
> +}
> +
>
> static void
> showError(virConnectPtr conn)
> @@ -31,23 +191,24 @@ showError(virConnectPtr conn)
> ret = virConnCopyLastError(conn, err);
>
> switch (ret) {
> - case 0:
> - snmp_log(LOG_ERR, "No error found\n");
> - break;
> -
> - case -1:
> - snmp_log(LOG_ERR, "Parameter error when attempting to get last error\n");
> - break;
> -
> - default:
> - snmp_log(LOG_ERR, "libvirt reported: \"%s\"\n", err->message);
> - break;
> + case 0:
> + snmp_log(LOG_ERR, "No error found\n");
> + break;
> +
> + case -1:
> + snmp_log(LOG_ERR,
> + "Parameter error when attempting to get last error\n");
> + break;
> +
> + default:
> + snmp_log(LOG_ERR, "libvirt reported: \"%s\"\n", err->message);
> + break;
> }
>
> virResetError(err);
> free(err);
>
> -out:
> + out:
> return;
> }
>
> @@ -55,7 +216,7 @@ out:
> * Populate libvirtGuestTable into given container.
> */
> int
> -libvirtSnmpLoadGuests(netsnmp_container *container)
> +libvirtSnmpLoadGuests(netsnmp_container * container)
> {
> int ret = 0, i, numIds, numActiveDomains;
> int *idList = NULL;
> @@ -80,9 +241,7 @@ libvirtSnmpLoadGuests(netsnmp_container *container)
> goto out;
> }
>
> - numIds = virConnectListDomains(conn,
> - idList,
> - numActiveDomains);
> + numIds = virConnectListDomains(conn, idList, numActiveDomains);
gratuitous (but understandable :-) whitespace change.
>
> if (-1 == numIds) {
> ret = -1;
> @@ -91,14 +250,14 @@ libvirtSnmpLoadGuests(netsnmp_container *container)
> goto out;
> }
>
> - for (i = 0 ; i< numIds ; i++) {
> - unsigned char uuid[16];
> + for (i = 0; i< numIds; i++) {
> + unsigned char uuid[16];
>
> domain = virDomainLookupByID(conn, *(idList + i));
> if (NULL == domain) {
> printf("Failed to lookup domain\n");
> showError(conn);
> - ret = -1;
> + ret = -1;
> goto out;
> }
Likewise - everything above is just formatting changes.
>
> @@ -106,47 +265,49 @@ libvirtSnmpLoadGuests(netsnmp_container *container)
> printf("Failed to get domain info\n");
> showError(conn);
> virDomainFree(domain);
> - ret = -1;
> + ret = -1;
> goto out;
> }
>
> /* create new row in the container */
> row_ctx = libvirtGuestTable_allocate_rowreq_ctx(NULL);
> if (!row_ctx) {
> - virDomainFree(domain);
> - snmp_log(LOG_ERR, "Error creating row");
> - ret = -1;
> - goto out;
> + virDomainFree(domain);
> + snmp_log(LOG_ERR, "Error creating row");
> + ret = -1;
> + goto out;
Okay, I'm seeing a pattern here. There are a *lot* of whitespace-only
changes in this file, which obscures the actual functional change. Could
you split out the whitespace changes into a separate patch? This will
not only make it easier to see what's being changed now, but in the
future if there was a bug in the code that was located via a git bisect,
the amount of lines to study would be dramatically reduced. (Yeah, I
know this is just an example, but it's a good principle to follow anyway.)
> }
> /* set the index of the row */
> ret = virDomainGetUUID(domain, uuid);
> if (ret) {
> - virDomainFree(domain);
> - snmp_log(LOG_ERR, "Cannot get UUID");
> - libvirtGuestTable_release_rowreq_ctx(row_ctx);
> - ret = -1;
> - goto out;
> - }
> - if (MFD_SUCCESS != libvirtGuestTable_indexes_set(row_ctx, (char*) uuid,
> - sizeof(uuid))) {
> - virDomainFree(domain);
> - snmp_log(LOG_ERR, "Error setting row index");
> - libvirtGuestTable_release_rowreq_ctx(row_ctx);
> - ret = -1;
> - goto out;
> + virDomainFree(domain);
> + snmp_log(LOG_ERR, "Cannot get UUID");
> + libvirtGuestTable_release_rowreq_ctx(row_ctx);
> + ret = -1;
> + goto out;
> + }
> + if (MFD_SUCCESS !=
> + libvirtGuestTable_indexes_set(row_ctx, (char *) uuid,
> + sizeof(uuid))) {
> + virDomainFree(domain);
> + snmp_log(LOG_ERR, "Error setting row index");
> + libvirtGuestTable_release_rowreq_ctx(row_ctx);
> + ret = -1;
> + goto out;
> }
>
> /* set the data */
> name = virDomainGetName(domain);
> if (name)
> - row_ctx->data.libvirtGuestName = strdup(name);
> + row_ctx->data.libvirtGuestName = strdup(name);
> else
> - row_ctx->data.libvirtGuestName = strdup("");
> + row_ctx->data.libvirtGuestName = strdup("");
> if (!row_ctx->data.libvirtGuestName) {
> - snmp_log(LOG_ERR, "Not enough memory for domain name '%s'", name);
> - libvirtGuestTable_release_rowreq_ctx(row_ctx);
> - ret = -1;
> - goto out;
> + snmp_log(LOG_ERR, "Not enough memory for domain name '%s'",
> + name);
> + libvirtGuestTable_release_rowreq_ctx(row_ctx);
> + ret = -1;
> + goto out;
> }
>
> row_ctx->data.libvirtGuestState = info.state;
> @@ -162,25 +323,133 @@ libvirtSnmpLoadGuests(netsnmp_container *container)
>
> ret = CONTAINER_INSERT(container, row_ctx);
> if (ret) {
> - snmp_log(LOG_ERR, "Cannot insert domain '%s' to container", name);
> - libvirtGuestTable_release_rowreq_ctx(row_ctx);
> - ret = -1;
> - goto out;
> - }
> + snmp_log(LOG_ERR, "Cannot insert domain '%s' to container",
> + name);
> + libvirtGuestTable_release_rowreq_ctx(row_ctx);
> + ret = -1;
> + goto out;
> + }
>
> }
>
> -out:
> + out:
> free(idList);
> return ret;
> }
>
> -int libvirtSnmpInit(void)
> +/* Polling thread function */
> +void *
> +pollingThreadFunc(void *foo)
> {
> + int sts;
> +
> + while (run) {
> + struct pollfd pfd = {.fd = h_fd,
> + .events = h_event,
> + .revents = 0
> + };
> +
> + sts = poll(&pfd, 1, TIMEOUT_MS);
> +
> + /* if t_timeout< 0 then t_cb must not be called */
> + if (t_cb&& t_active&& t_timeout>= 0) {
> + t_cb(t_timeout, t_opaque);
> + }
> +
> + if (sts == 0) {
> + /* DEBUG0("Poll timeout"); */
> + continue;
> + }
> + if (sts< 0) {
> + DEBUG0("Poll failed");
> + continue;
> + }
> + if (pfd.revents& POLLHUP) {
> + DEBUG0("Reset by peer");
> + pthread_exit(NULL);
> + }
> +
> + if (h_cb) {
> + h_cb(0,
> + h_fd,
> + myPollEventToEventHandleType(pfd.revents& h_event),
> + h_opaque);
> + }
> +
> + }
> +
> + pthread_exit(NULL);
> +}
> +
> +/* Function to register domain lifecycle events collection */
> +int
> +libvirtRegisterEvents(virConnectPtr conn)
> +{
> + struct sigaction action_stop;
> + pthread_attr_t thread_attr;
> +
> + memset(&action_stop, 0, sizeof action_stop);
> +
> + action_stop.sa_handler = stop;
> +
> + sigaction(SIGTERM,&action_stop, NULL);
> + sigaction(SIGINT,&action_stop, NULL);
> +
> + DEBUG0("Registering domain event callback");
> +
> + callbackRet = virConnectDomainEventRegisterAny(conn, NULL,
> + VIR_DOMAIN_EVENT_ID_LIFECYCLE,
> + VIR_DOMAIN_EVENT_CALLBACK
> + (domainEventCallback),
> + NULL, myFreeFunc);
> +
> + if (callbackRet == -1)
> + return -1;
> +
> + /* we need a thread to poll for events */
> + pthread_attr_init(&thread_attr);
> + pthread_attr_setdetachstate(&thread_attr, PTHREAD_CREATE_JOINABLE);
> +
> + if (pthread_create
> + (&poll_thread,&thread_attr, pollingThreadFunc, NULL))
> + return -1;
> +
> + pthread_attr_destroy(&thread_attr);
> +
> + return 0;
> +}
> +
> +/* Unregister domain events collection */
> +int
> +libvirtUnregisterEvents(virConnectPtr conn)
> +{
> + void *status;
> +
> + pthread_join(poll_thread,&status);
> + virConnectDomainEventDeregisterAny(conn, callbackRet);
> + callbackRet = -1;
> + return 0;
> +}
> +
> +int
> +libvirtSnmpInit(void)
> +{
> + char *verbose_env = getenv("VERBOSE");
> +
> + verbose = verbose_env != NULL;
> +
> + /* if we don't already have registered callback */
> + if (callbackRet == -1)
> + virEventRegisterImpl(myEventAddHandleFunc,
> + myEventUpdateHandleFunc,
> + myEventRemoveHandleFunc,
> + myEventAddTimeoutFunc,
> + myEventUpdateTimeoutFunc,
> + myEventRemoveTimeoutFunc);
> +
> /* virConnectOpenAuth is called here with all default parameters,
> - * except, possibly, the URI of the hypervisor. */
> - /* TODO: configure the URI */
> - /* Use libvirt env variable LIBVIRT_DEFAULT_URI by default*/
> + * /* TODO: configure the URI */
> + /* Use libvirt env variable LIBVIRT_DEFAULT_URI by default */
> conn = virConnectOpenAuth(NULL, virConnectAuthPtrDefault, 0);
>
> if (NULL == conn) {
> @@ -188,11 +457,22 @@ int libvirtSnmpInit(void)
> showError(conn);
> return -1;
> }
> +
> + if ((callbackRet == -1)&& libvirtRegisterEvents(conn)) {
> + printf("Unable to register domain events\n");
> + return -1;
> + }
> +
> return 0;
> }
>
> -void libvirtSnmpShutdown(void)
> +void
> +libvirtSnmpShutdown(void)
> {
> + if (libvirtUnregisterEvents(conn)) {
> + printf("Failed to unregister domain events\n");
> + }
> +
> if (0 != virConnectClose(conn)) {
> printf("Failed to disconnect from hypervisor\n");
> showError(conn);
> @@ -202,95 +482,98 @@ void libvirtSnmpShutdown(void)
> int
> libvirtSnmpCheckDomainExists(unsigned char *uuid)
> {
> - virDomainPtr d = virDomainLookupByUUID(conn, uuid);
> - if (d == NULL)
> - return -1;
> + virDomainPtr d = virDomainLookupByUUID(conn, uuid);
>
> - virDomainFree(d);
> - return 0;
> + if (d == NULL)
> + return -1;
> +
> + virDomainFree(d);
> + return 0;
> }
>
> int
> libvirtSnmpCreate(unsigned char *uuid, int state)
> {
> - virDomainPtr dom;
> - int ret;
> - unsigned int flags = 0;
> -
> - dom = virDomainLookupByUUID(conn, uuid);
> - if (dom == NULL) {
> - printf("Cannot find domain to create\n");
> - return -1;
> - }
> -
> - switch(state) {
> - case VIR_DOMAIN_RUNNING:
> - flags = 0;
> - break;
> - case VIR_DOMAIN_PAUSED:
> - flags = VIR_DOMAIN_START_PAUSED;
> - break;
> - default:
> - printf("Can't create domain with state %d\n", flags);
> - return -1;
> - }
> -
> - ret = virDomainCreateWithFlags(dom, flags);
> - if (ret) {
> - showError(conn);
> - }
> - virDomainFree(dom);
> - return ret;
> + virDomainPtr dom;
> + int ret;
> + unsigned int flags = 0;
> +
> + dom = virDomainLookupByUUID(conn, uuid);
> + if (dom == NULL) {
> + printf("Cannot find domain to create\n");
> + return -1;
> + }
> +
> + switch (state) {
> + case VIR_DOMAIN_RUNNING:
> + flags = 0;
> + break;
> + case VIR_DOMAIN_PAUSED:
> + flags = VIR_DOMAIN_START_PAUSED;
> + break;
> + default:
> + printf("Can't create domain with state %d\n", flags);
> + return -1;
> + }
> +
> + ret = virDomainCreateWithFlags(dom, flags);
> + if (ret) {
> + showError(conn);
> + }
> + virDomainFree(dom);
> + return ret;
> }
>
> int
> libvirtSnmpDestroy(unsigned char *uuid)
> {
> - virDomainPtr dom;
> - int ret;
> -
> - dom = virDomainLookupByUUID(conn, uuid);
> - if (dom == NULL) {
> - printf("Cannot find domain to destroy\n");
> - return -1;
> - }
> -
> - ret = virDomainDestroy(dom);
> - if (ret) {
> - showError(conn);
> - }
> - virDomainFree(dom);
> - return ret;
> + virDomainPtr dom;
> + int ret;
> +
> + dom = virDomainLookupByUUID(conn, uuid);
> + if (dom == NULL) {
> + printf("Cannot find domain to destroy\n");
> + return -1;
> + }
> +
> + ret = virDomainDestroy(dom);
> + if (ret) {
> + showError(conn);
> + }
> + virDomainFree(dom);
> + return ret;
> }
>
> int
> libvirtSnmpChangeState(unsigned char *uuid, int newstate, int oldstate)
> {
> - virDomainPtr dom;
> - int ret = 0;
> -
> - dom = virDomainLookupByUUID(conn, uuid);
> - if (dom == NULL) {
> - printf("Cannot find domain to change\n");
> - return 1;
> - }
> -
> - if (oldstate == VIR_DOMAIN_RUNNING&& newstate == VIR_DOMAIN_PAUSED)
> - ret = virDomainSuspend(dom);
> - else if (oldstate == VIR_DOMAIN_PAUSED&& newstate == VIR_DOMAIN_RUNNING)
> - ret = virDomainResume(dom);
> - else if (newstate == VIR_DOMAIN_SHUTDOWN)
> - ret = virDomainShutdown(dom);
> - else {
> - printf("Wrong state transition from %d to %d\n", oldstate, newstate);
> - ret = -1;
> - goto out;
> - }
> -
> - if (ret != 0) {
> - showError(conn);
> - }
> -out:
> - virDomainFree(dom);
> - return ret;
> + virDomainPtr dom;
> + int ret = 0;
> +
> + dom = virDomainLookupByUUID(conn, uuid);
> + if (dom == NULL) {
> + printf("Cannot find domain to change\n");
> + return 1;
> + }
> +
> + if (oldstate == VIR_DOMAIN_RUNNING&& newstate == VIR_DOMAIN_PAUSED)
> + ret = virDomainSuspend(dom);
> + else if (oldstate == VIR_DOMAIN_PAUSED
> +&& newstate == VIR_DOMAIN_RUNNING)
> + ret = virDomainResume(dom);
> + else if (newstate == VIR_DOMAIN_SHUTDOWN)
> + ret = virDomainShutdown(dom);
> + else {
> + printf("Wrong state transition from %d to %d\n", oldstate,
> + newstate);
> + ret = -1;
> + goto out;
> + }
> +
> + if (ret != 0) {
> + showError(conn);
> + }
> + out:
> + virDomainFree(dom);
> + return ret;
> }
> diff --git a/src/libvirtSnmp.h b/src/libvirtSnmp.h
> index 4ac6130..5812c91 100644
This file seems to be all whitespace changes.
> --- a/src/libvirtSnmp.h
> +++ b/src/libvirtSnmp.h
> @@ -14,29 +14,28 @@
> * Populate libvirtGuestTable into given container.
> */
> extern int
> -libvirtSnmpLoadGuests(netsnmp_container *container);
> + libvirtSnmpLoadGuests(netsnmp_container * container);
>
> extern int
> -libvirtSnmpInit(void);
> + libvirtSnmpInit(void);
>
> extern void
> -libvirtSnmpShutdown(void);
> + libvirtSnmpShutdown(void);
>
> /**
> * Check that domain with given UUID exists.
> * Return 0 if so, -1 if not.
> */
> extern int
> -libvirtSnmpCheckDomainExists(unsigned char *uuid);
> + libvirtSnmpCheckDomainExists(unsigned char *uuid);
>
> extern int
> -libvirtSnmpCreate(unsigned char *uuid, int state);
> + libvirtSnmpCreate(unsigned char *uuid, int state);
>
> extern int
> -libvirtSnmpDestroy(unsigned char *uuid);
> + libvirtSnmpDestroy(unsigned char *uuid);
>
> extern int
> -libvirtSnmpChangeState(unsigned char *uuid, int newstate, int oldstate);
> + libvirtSnmpChangeState(unsigned char *uuid, int newstate, int oldstate);
>
> #endif /* __VIR_SNMP_H__ */
> -
More information about the libvir-list
mailing list