pcp
[Top] [All Lists]

Re: [pcp] IB pmda and writing out the default config file

To: Martin Hicks <mort@xxxxxxx>
Subject: Re: [pcp] IB pmda and writing out the default config file
From: Mark Goodwin <goodwinos@xxxxxxxxx>
Date: Tue, 14 Jul 2009 21:01:55 +1000
Cc: Max Matveev <makc@xxxxxxxxx>, pcp@xxxxxxxxxxx
In-reply-to: <20090706173919.GC18171@xxxxxxxxxxxxxxxxxxxxxxxxx>
References: <20090608171828.GA14199@xxxxxxxxxxxxxxxxxxxxxxxxx> <18990.24528.172761.247069@xxxxxxxxxxxx> <20090616182224.GI20783@xxxxxxxxxxxxxxxxxxxxxxxxx> <20090706173919.GC18171@xxxxxxxxxxxxxxxxxxxxxxxxx>
User-agent: Thunderbird 2.0.0.21 (X11/20090320)
Martin Hicks wrote:
How about this patch.  It rejigs things a lot, but essentially:

- No config file will be created unless the '-w' flag is passed to
  pmdaib. (this is a change)

- By default, if no config file is found, only local ports are monitored.
  (before we would always write out a config file containing only local
   ports)

- If a config file exists then it is the preferred source of information
  and perhaps not even local ports will be monitored. (before you could
  have edited the automatically created file to remove the local ports)


So in the case of a user who is using a custom config file (either to
name the instances differently, or to monitor remote ports) who replaces
an HCA, it will be up to them to edit the config file that they created.

For those who are using the ib pmda in its simplest mode with no config
file, no changes will be necessary when an HCA is replaced but they have
no guarantee of static device naming if they add an additional HCA.

Seems like a reasonable compromise to me. Instance naming in merged
archives spanning reboots will sometimes be screwed up, but you still
have the option to avoid that if it really matters. When you look at the
flaming hoops the scsi and device-mapper folks have jumped thru to
achieve persistent naming, a more sophisticated scheme isn't warranted; set-and-forget performance monitoring is more important in most cases.

Cheers
-- Mark

diff --git a/src/pmdas/ib/daemon.c b/src/pmdas/ib/daemon.c
index 2a71151..12540cc 100644
--- a/src/pmdas/ib/daemon.c
+++ b/src/pmdas/ib/daemon.c
@@ -33,7 +33,8 @@ usage(void)
     fputs("Options:\n"
           "  -d domain  use domain (numeric) for metrics domain of PMDA\n"
          "  -l logfile write log into logfile rather than using default log 
name\n"
-         "  -c path to configuration file\n",
+         "  -c path to configuration file\n"
+         "  -w write the basic configuration file\n",
           stderr);
     exit(1);
 }
@@ -46,19 +47,22 @@ main(int argc, char **argv)
     pmdaInterface dispatch;
     char helppath[MAXPATHLEN];
     char *confpath = NULL;
-    char *p;
     int opt;
+    int writeconf = 0;
__pmSetProgname(argv[0]); snprintf(helppath, sizeof(helppath), "%s%c" "ib" "%c" "help", pmGetConfig("PCP_PMDAS_DIR"), sep, sep);
     pmdaDaemon(&dispatch, PMDA_INTERFACE_3, pmProgname, IB, "ib.log", 
helppath);
- while ((opt = pmdaGetOpt(argc, argv, "D:c:d:l:?", &dispatch, &err)) != EOF) {
+    while ((opt = pmdaGetOpt(argc, argv, "D:c:d:l:w?", &dispatch, &err)) != 
EOF) {
        switch (opt) {
        case 'c':
            confpath = optarg;
            break;
+       case 'w':
+           writeconf = 1;
+           break;
        default:
            err++;
        }
@@ -69,7 +73,7 @@ main(int argc, char **argv)
     }
pmdaOpenLog(&dispatch);
-    ibpmda_init(confpath, &dispatch);
+    ibpmda_init(confpath, writeconf, &dispatch);
     pmdaConnect(&dispatch);
     pmdaMain(&dispatch);
diff --git a/src/pmdas/ib/dso.c b/src/pmdas/ib/dso.c
index 33f10a8..4e506f8 100644
--- a/src/pmdas/ib/dso.c
+++ b/src/pmdas/ib/dso.c
@@ -37,5 +37,5 @@ ib_init (pmdaInterface * dispatch)
             pmGetConfig("PCP_PMDAS_DIR"), sep, sep);
     pmdaDSO(dispatch, PMDA_INTERFACE_3, "ibpmda", helppath);
- ibpmda_init(NULL, dispatch);
+    ibpmda_init(NULL, 0, dispatch);
 }
diff --git a/src/pmdas/ib/ib.c b/src/pmdas/ib/ib.c
index ccfb079..c027f77 100644
--- a/src/pmdas/ib/ib.c
+++ b/src/pmdas/ib/ib.c
@@ -141,6 +141,70 @@ typedef struct port_state_s {
char pcap[IB_ALLPORTCAPSTRLEN]; } port_state_t; +static char confpath[MAXPATHLEN];
+static int portcount;
+/* Line number while parsing the config file */
+static FILE *fconf;
+static int lcnt;
+
+#define print_parse_err(loglevel, fmt, args...) \
+    if (fconf) { \
+       __pmNotifyErr(loglevel, "%s(%d): " fmt, fconf, lcnt, args); \
+    } else { \
+       __pmNotifyErr(loglevel, fmt, args); \
+    }
+
+static void
+monitor_guid(pmdaIndom *itab, char *name, long long guid, int rport,
+           char *local, int lport)
+{
+    int inst;
+    hca_state_t *hca = NULL;
+    port_state_t *ps;
+ + if (pmdaCacheLookupName(itab[IB_HCA_INDOM].it_indom, local, NULL, + (void**)&hca) != PMDA_CACHE_ACTIVE) {
+       print_parse_err(LOG_ERR, "unknown HCA '%s' in 'via' clause\n", local);
+       return;
+    }
+
+    if ((lport >= UMAD_CA_MAX_PORTS) || (lport < 0)) {
+ print_parse_err(LOG_ERR, + "port number %d is out of bounds for HCA %s\n",
+                       lport, local);
+       return;
+    }
+ + if (hca->lports[lport].hndl == NULL) { + print_parse_err(LOG_ERR, + "port %s:%d has failed initialization\n",
+                       local, lport);
+       return;
+    }
+ + if ((ps = (port_state_t *)calloc(1, sizeof(port_state_t))) == NULL) {
+       __pmNotifyErr (LOG_ERR, "Out of memory to save state for %s\n", name);
+       return;
+    }
+
+    ps->guid = guid;
+    ps->remport = rport;
+    ps->lport = hca->lports + lport;
+    ps->portid.lid = -1;
+    ps->timeout = 1000;
+
+    if ((inst = pmdaCacheStore(itab[IB_PORT_INDOM].it_indom,
+                           PMDA_CACHE_ADD, name, ps)) < 0) {
+       __pmNotifyErr(LOG_ERR, "Cannot add %s to the cache - %s\n",
+                       name, pmErrStr(inst));
+       free (ps);
+       return;
+    }
+
+    portcount++;
+}
+
+
 static int
 foreachport(hca_state_t *hst, void (*cb)(hca_state_t *, umad_port_t *, void *),
            void *closure)
@@ -166,7 +230,6 @@ foreachport(hca_state_t *hst, void (*cb)(hca_state_t *, 
umad_port_t *, void *),
 static void
 printportconfig (hca_state_t *hst, umad_port_t *port, void *arg)
 {
-    FILE *fconf = arg;
     uint64_t hguid = port->port_guid;
__ntohll((char *)&hguid);
@@ -176,6 +239,20 @@ printportconfig (hca_state_t *hst, umad_port_t *port, void 
*arg)
             hst->ca.ca_name, port->portnum);
 }
+static void
+monitorport(hca_state_t *hst, umad_port_t *port, void *arg)
+{
+    pmdaIndom *itab = arg;
+    uint64_t hguid = port->port_guid;
+    char name[128];
+
+    __ntohll((char *)&hguid);
+    sprintf(name, "%s:%d", port->ca_name, port->portnum);
+
+    monitor_guid(itab, name, hguid, port->portnum, port->ca_name, 
port->portnum);
+}
+
+
static int mgmt_classes[] = {IB_SMI_CLASS, IB_SMI_DIRECT_CLASS, IB_SA_CLASS, IB_PERFORMANCE_CLASS};
 static void
@@ -196,74 +273,10 @@ openumadport (hca_state_t *hst, umad_port_t *port, void 
*arg)
     lp->hndl = hndl;
 }
-int
-ib_load_config(const char *confpath, pmdaIndom *itab, unsigned int nindoms)
+static void
+parse_config(pmdaIndom *itab)
 {
-    char hcas[IBPMDA_MAX_HCAS][UMAD_CA_NAME_LEN];
-    hca_state_t *st = NULL;
-    int i, n;
-    int lcnt = 0;
     char buffer[2048];
-    FILE *fconf;
-    int (*closef)(FILE *) = fclose;
-    int portcount = 0;
-
-    if (nindoms <= IB_CNT_INDOM) {
-       return (-EINVAL);
-    }  
-
-    if (umad_init()) {
-       return (-EIO);
- } -
-    if ((n = umad_get_cas_names(hcas, ARRAYSZ(hcas)))) {
-       if ((st = calloc (n, sizeof(hca_state_t))) == NULL) {
-           return (-ENOMEM);
-       }
-    } else {
-       return (0);
-    }
-
-    for (i=0; i < n; i++) {
-       if (umad_get_ca(hcas[i], &st[i].ca) == 0) {
-           int e = pmdaCacheStore (itab[IB_HCA_INDOM].it_indom, PMDA_CACHE_ADD,
-                                   st[i].ca.ca_name, &st[i].ca);
-
-           if (e >= 0) {
-               foreachport (st+i, openumadport, NULL);
-           } else {
- __pmNotifyErr (LOG_ERR, - "Cannot add instance for %s to the "
-                               "cache - %s\n",
-                                st[i].ca.ca_name, pmErrStr(e));
-           }
-       }
-    }
-
-    /* Process config file - if the executable bit is set then assume
-     * that user wants it to be a script and run it, otherwise try loading
- * it. If it cannot be loaded then create a default one and - * process it again */
-    if (access(confpath, X_OK)) {
-       /* Not an executable, just read it */
-       if ((fconf = fopen (confpath, "r")) == NULL) {
-           if ((fconf = fopen (confpath, "w+")) != NULL) {
-               for (i=0; i < n; i++) {
-                   foreachport (st+i, printportconfig, fconf);
-               }
-               fseek (fconf, 0, SEEK_SET);
-           }
-       }
-    } else {
-       fconf = popen(confpath, "r");
-       closef = pclose;
-    }
-
-    if (fconf == NULL) {
-       __pmNotifyErr (LOG_CRIT, "Cannot open configuration file %s\n",
-                       confpath);
-       return (-ENOENT);
-    }  
while ((fgets(buffer, sizeof(buffer)-1, fconf)) != NULL) {
        char *p;
@@ -285,64 +298,98 @@ ib_load_config(const char *confpath, pmdaIndom *itab, 
unsigned int nindoms)
            int rport;
            char local[128];
            int lport;
-           hca_state_t *hca = NULL;
-           port_state_t *ps;
if (sscanf(p, "%[^ \t]%llx%d via %[^:]:%d",
                        name, &guid, &rport, local, &lport) != 5) {
                __pmNotifyErr (LOG_ERR, "%s(%d): cannot parse the line\n",
                               confpath, lcnt);
-           } else if (pmdaCacheLookupName(itab[IB_HCA_INDOM].it_indom,
- local, NULL, - (void**)&hca) != PMDA_CACHE_ACTIVE) { - __pmNotifyErr (LOG_ERR, - "%s(%d): unknown HCA '%s' in 'via' clause\n",
-                              confpath, lcnt, local);
-           } else if ((lport >= UMAD_CA_MAX_PORTS) || (lport < 0)) {
- __pmNotifyErr (LOG_ERR, - "%s(%d): port number %d is out of bounds for "
-                              "HCA %s\n",
-                               confpath, lcnt, lport, local);
-           } else if (hca->lports[lport].hndl == NULL) {
- __pmNotifyErr (LOG_ERR, - "%s(%d): port %s:%d has failed initialization\n",
-                               confpath, lcnt, local, lport);
-           } else if ((ps = (port_state_t *)calloc(1, sizeof(port_state_t))) 
== NULL) {
-               __pmNotifyErr (LOG_ERR, "Out of memory to save state for %s\n",
-                               name);
-           } else {
-               int inst;
-               ps->guid = guid;
-               ps->remport = rport;
-               ps->lport = hca->lports + lport;
-               ps->portid.lid = -1;
-               ps->timeout = 1000;
-
-               if ((inst = pmdaCacheStore (itab[IB_PORT_INDOM].it_indom,
-                                           PMDA_CACHE_ADD, name, ps)) < 0) {
- __pmNotifyErr (LOG_ERR, - "Cannot add %s to the cache - %s\n",
-                                       name, pmErrStr(inst));
-                       free (ps);
-               } else {
-                       portcount++;
-               }
+               continue;
            }
+ + monitor_guid(itab, name, guid, rport, local, lport);
        }
     }
-    (*closef) (fconf);
+}
+
+int
+ib_load_config(const char *cp, int writeconf, pmdaIndom *itab, unsigned int 
nindoms)
+{
+    char hcas[IBPMDA_MAX_HCAS][UMAD_CA_NAME_LEN];
+    hca_state_t *st = NULL;
+    int i, n;
+    int (*closef)(FILE *) = fclose;
+
+    if (nindoms <= IB_CNT_INDOM)
+       return -EINVAL;
+
+    if (umad_init())
+       return -EIO;
+
+    if ((n = umad_get_cas_names(hcas, ARRAYSZ(hcas)))) {
+       if ((st = calloc (n, sizeof(hca_state_t))) == NULL)
+           return -ENOMEM;
+    } else
+       /* No HCAs */
+       return 0;
+
+    /* Open config file - if the executable bit is set then assume that
+     * user wants it to be a script and run it, otherwise try loading it.
+     */
+    strcpy(confpath, cp);
+    if (access(confpath, F_OK) == 0) {
+       if (writeconf) {
+           __pmNotifyErr(LOG_ERR,
+                   "Config file exists and writeconf arg was given to pmdaib.  
"
+                   "Using existing config file");
+           writeconf = 0;
+       }
+
+       if (access(confpath, X_OK)) {
+           /* Not an executable, just read it */
+           fconf = fopen (confpath, "r");
+       } else if (!writeconf) {
+           fconf = popen(confpath, "r");
+           closef = pclose;
+ } + } else if (writeconf) {
+       fconf = fopen(confpath, "w");
+    }
+
+    for (i=0; i < n; i++) {
+       if (umad_get_ca(hcas[i], &st[i].ca) == 0) {
+           int e = pmdaCacheStore(itab[IB_HCA_INDOM].it_indom, PMDA_CACHE_ADD,
+                                   st[i].ca.ca_name, &st[i].ca);
+
+           if (e < 0) {
+ __pmNotifyErr(LOG_ERR, + "Cannot add instance for %s to the cache - %s\n",
+                        st[i].ca.ca_name, pmErrStr(e));
+               continue;
+           }
+
+           foreachport(st+i, openumadport, NULL);
+           if (fconf == NULL || writeconf)
+               /* No config file - monitor local ports */
+               foreachport(st+i, monitorport, itab);
+           if (writeconf)
+               foreachport(st+i, printportconfig, fconf);
+       }
+    }
+
+    if (fconf) {
+       parse_config(itab);
+       (*closef)(fconf);
+    }
if (!portcount) { - __pmNotifyErr (LOG_INFO, - "No ports found in configuration file %s\n",
-                      confpath);
+       __pmNotifyErr(LOG_INFO, "No IB ports found to monitor");
     }
-               
+
itab[IB_CNT_INDOM].it_set = (pmdaInstid *)calloc(ARRAYSZ(mad_cnt_descriptors), sizeof(pmdaInstid)); if (itab[IB_CNT_INDOM].it_set == NULL) {
-       return (-ENOMEM);
+       return -ENOMEM;
     }
itab[IB_CNT_INDOM].it_numinst = ARRAYSZ(mad_cnt_descriptors);
@@ -351,8 +398,8 @@ ib_load_config(const char *confpath, pmdaIndom *itab, 
unsigned int nindoms)
        itab[IB_CNT_INDOM].it_set[i].i_name = mad_cnt_descriptors[i].name;
}
-       
-    return (0);
+
+    return 0;
 }
static char *
@@ -869,7 +916,7 @@ ib_store(pmResult *result, pmdaExt *pmda)
                    return (PM_ERR_INST);
                }
                break;
- +
            case METRIC_ib_control_hiwat:
                if ((id < 0) ||
                    (id > pmda->e_indoms[IB_CNT_INDOM].it_numinst)) {
@@ -884,7 +931,7 @@ ib_store(pmResult *result, pmdaExt *pmda)
            }
         }
     }
-    return (0);
+    return 0;
 }
diff --git a/src/pmdas/ib/ibpmda.h b/src/pmdas/ib/ibpmda.h
index 153743b..fae2c3c 100644
--- a/src/pmdas/ib/ibpmda.h
+++ b/src/pmdas/ib/ibpmda.h
@@ -22,10 +22,10 @@
 #include "domain.h"
-void ibpmda_init (const char *configpath, pmdaInterface *);
+void ibpmda_init (const char *configpath, int, pmdaInterface *);
int ib_fetch_val(pmdaMetric *, unsigned int, pmAtomValue *);
-int ib_load_config(const char *, pmdaIndom *, unsigned int);
+int ib_load_config(const char *, int, pmdaIndom *, unsigned int);
 void ib_rearm_for_update(void *);
 void ib_reset_perfcounters (void *);
 int ib_store(pmResult *, pmdaExt *);
diff --git a/src/pmdas/ib/pmda.c b/src/pmdas/ib/pmda.c
index 6a8e02d..d5557b3 100644
--- a/src/pmdas/ib/pmda.c
+++ b/src/pmdas/ib/pmda.c
@@ -297,7 +297,7 @@ ib_fetch(int numpmid, pmID pmidlist[], pmResult **resp, 
pmdaExt *pmda)
 }
void
-ibpmda_init(const char *confpath, pmdaInterface *dp)
+ibpmda_init(const char *confpath, int writeconf, pmdaInterface *dp)
 {
     char defconf[MAXPATHLEN];
     int sep = __pmPathSeparator();
@@ -320,7 +320,7 @@ ibpmda_init(const char *confpath, pmdaInterface *dp)
     }
- if ((dp->status = ib_load_config (confpath, indomtab, ARRAYSZ(indomtab))))
+    if ((dp->status = ib_load_config(confpath, writeconf, indomtab, 
ARRAYSZ(indomtab))))
        return;
for (i=0; i < ARRAYSZ(indomtab); i++) {


_______________________________________________
pcp mailing list
pcp@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/pcp

_______________________________________________
pcp mailing list
pcp@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/pcp

<Prev in Thread] Current Thread [Next in Thread>