[libvirt] [libvirt-snmp][PATCH v3 2/3] Create functions to fill in and send notification packets.

[Daniel P. Berrange]

On Wed, Mar 23, 2011 at 11:06:07AM +0100, Michal Privoznik wrote:
> Before sending snmp notification we need to fill in useful data,
> like domain status, UUID, etc.
> ---
>  src/Makefile.am            |    2 +
>  src/README.txt             |    7 ++-
>  src/libvirtNotifications.c |  109 ++++++++++++++++++++++++++++++++++++++++++++
>  src/libvirtNotifications.h |   21 ++++++++
>  4 files changed, 137 insertions(+), 2 deletions(-)
>  create mode 100644 src/libvirtNotifications.c
>  create mode 100644 src/libvirtNotifications.h
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index c781e230e6b5f87c6776f40360d580ff0b50fc2b..d68f17499d8768706050a7f38f77fc597fb0db46 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -15,12 +15,14 @@ USER_SRCS = 	\
>  		libvirtGuestTable_data_get.c \
>  		libvirtGuestTable_data_set.c \
>  		libvirtGuestTable_data_access.c \
> +		libvirtNotifications.c \
>  		libvirtSnmp.c
>  
>  USER_HDRS = 	\
>  		libvirtGuestTable_data_get.h \
>  		libvirtGuestTable_data_set.h \
>  		libvirtGuestTable_data_access.h \
> +		libvirtNotifications.h \
>  		libvirtSnmp.h
>  
>  SRCS = 		\
> diff --git a/src/README.txt b/src/README.txt
> index 22dd2af8436de1797afcee37f47218fe46dac4fd..6d010f6d0fedfa6e47333468bfa9dec40e47d29c 100644
> --- a/src/README.txt
> +++ b/src/README.txt
> @@ -31,8 +31,11 @@ libvirtGuestTable*
>  - Generated by this command: 'MIBDIRS="+." MIBS="+LIBVIRT-MIB" mib2c libvirtMIB'.
>    - It asks few questions and produces the mentioned files.
>  - libvirtGuestTable.h, libvirtGuestTable.c, libvirtGuestTable_data_access.c, libvirtGuestTable_data_get.c and libvirtGuestTable_data_set.c are manually edited to set correct data structures and their handling. It's nicely documented, see libvirtGuestTable-README-FIRST.txt and libvirtGuestTable-README-libvirtGuestTable.txt + TODOs in the code. It took me ~2 hours to update these files (but I've done this several times for different MIBs, so I know what to do :). Do not touch the rest of the files!
> -- 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).
>  
>  
>  Usage (tested on Fedora 14 and RHEL6)
> diff --git a/src/libvirtNotifications.c b/src/libvirtNotifications.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..0001425b0c1c338bca01d958320ee2379d792866
> --- /dev/null
> +++ b/src/libvirtNotifications.c
> @@ -0,0 +1,109 @@
> +/*
> + * libvirtNotifications.c: Fill in trap packets
> + *
> + * Copyright (C) 2011 Red Hat, Inc.
> + *
> + * See COPYING for the license of this software
> + *
> + * Michal Privoznik <mprivozn at redhat.com>
> + */
> +
> +#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_BUFLEN];
> +    virDomainInfo info;
> +    int rowstatus = ROWSTATUS_ACTIVE;
> +
> +    if (virDomainGetUUID(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;
> +    }

In the error cases, I thuink you really want to be printing
the message from the virErrorPtr object, so the admin or
bug triager can diagnose what is causing the failure.

I'd suggest creating a helper method for printing the
error, eg

  static void printLibvirtError(const char *msg) {
      virErrorPtr err = virGetLastError();

      fprintf(stderr, "%s: %s", msg, err ? err->message : "Unknown error");
  }

Also when checking for failure, you should check
for   "< 0" or  "!= NULL", since positive return
values are used for non-error conditions.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|