(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__ */ -