pcp
[Top] [All Lists]

Re: [pcp] libvirt pmda: support old libvirt 0.10 API

To: Nathan Scott <nathans@xxxxxxxxxx>
Subject: Re: [pcp] libvirt pmda: support old libvirt 0.10 API
From: Marko Myllynen <myllynen@xxxxxxxxxx>
Date: Fri, 15 Jul 2016 13:35:40 +0300
Cc: pcp developers <pcp@xxxxxxxxxxx>
Delivered-to: pcp@xxxxxxxxxxx
In-reply-to: <1095970845.5980166.1468542456963.JavaMail.zimbra@xxxxxxxxxx>
Organization: Red Hat
References: <6abca560-398c-3a87-350d-c2cfba1a9b78@xxxxxxxxxx> <1095970845.5980166.1468542456963.JavaMail.zimbra@xxxxxxxxxx>
Reply-to: Marko Myllynen <myllynen@xxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2
Hi,

On 2016-07-15 03:27, Nathan Scott wrote:
> Hi Marko,
> ----- Original Message -----
>> [...]
>> This is mostly for RHEL 6 / libvirt-0.10 environments but since now
>> configurable, can be enabled anywhere where the newer API is not
>> an option for some reason.
> 
> I wonder if its possible to do this fully automatically?

It's being done automatically already, this knob just enables to
selecting either or (if available).

> i.e. not exposed to users at all in the config file).

Indeed it's perhaps not very helpful for most users. However, it's
only one option, and ...

> API for inspecting the libvirt version, if that helps?
> 
> $ python
>>>> import libvirt
>>>> print libvirt.getVersion()
> 1002018

... while the below patch uses something slightly more reliable and
also updates the connect helper to check this, I think this still does
not catch all cases (if no domains are running, we can't check whether
this actually works or not). It looks like the feature is currently
QEMU only (there seems to be a lot deviation in available functionality
in various libvirt drivers). Leaving the option visible at least in the
man page is probably justified.

(The patch also updates Install which was still using temp domain ID).

---
 src/pmdas/libvirt/Install            |  2 +-
 src/pmdas/libvirt/connect            | 16 ++++++++++++++--
 src/pmdas/libvirt/pmdalibvirt.python | 11 +++--------
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/pmdas/libvirt/Install b/src/pmdas/libvirt/Install
index d82614c..4a32867 100755
--- a/src/pmdas/libvirt/Install
+++ b/src/pmdas/libvirt/Install
@@ -19,7 +19,7 @@
 . $PCP_SHARE_DIR/lib/pmdaproc.sh
 
 iam=libvirt
-domain=491
+domain=140
 python_opt=true
 daemon_opt=false
 
diff --git a/src/pmdas/libvirt/connect b/src/pmdas/libvirt/connect
index 76cd6bf..b1f9f8a 100755
--- a/src/pmdas/libvirt/connect
+++ b/src/pmdas/libvirt/connect
@@ -28,6 +28,7 @@ from pcp.pmapi import pmContext as PCP
 conffile = PCP.pmGetConfig('PCP_PMDAS_DIR')
 conffile += '/libvirt/libvirt.conf'
 
+oldapi = False
 user = 'root'
 uri = 'qemu:///system'
 
@@ -35,7 +36,11 @@ config = ConfigParser.SafeConfigParser()
 config.read(conffile)
 if config.has_section('pmda'):
     for opt in config.options('pmda'):
-        if opt == 'user':
+        if opt == 'oldapi':
+            if config.get('pmda', opt) == 'True' or \
+            config.get('pmda', opt) == '1':
+                oldapi = True
+        elif opt == 'user':
             user = config.get('pmda', opt)
         elif opt == 'uri':
             uri = config.get('pmda', opt)
@@ -44,7 +49,7 @@ if config.has_section('pmda'):
             sys.exit(1)
 
 if len(sys.argv) > 1 and (sys.argv[1] == '-c' or sys.argv[1] == '--config'):
-    sys.stdout.write("user=%s\nuri=%s\n" % (user, uri))
+    sys.stdout.write("oldapi=%s\nuser=%s\nuri=%s\n" % (oldapi, user, uri))
     sys.exit(0)
 
 try:
@@ -62,3 +67,10 @@ except:
     sys.exit(1)
 
 sys.stdout.write("Connection as %s to %s ok.\n" % (user, uri))
+
+newapi = False
+if 'domainListGetStats' in (dir(conn)):
+    newapi = True
+else:
+    oldapi = True
+sys.stdout.write("Using new API: %s (available: %s).\n" % (not oldapi, newapi))
diff --git a/src/pmdas/libvirt/pmdalibvirt.python 
b/src/pmdas/libvirt/pmdalibvirt.python
index 2964d3d..137138b 100755
--- a/src/pmdas/libvirt/pmdalibvirt.python
+++ b/src/pmdas/libvirt/pmdalibvirt.python
@@ -57,9 +57,7 @@ class LibvirtPMDA(PMDA):
         self.doms = []
         self.connect_pmcd()
         self.conn = self.connect_libvirt()
-        try:
-            test = libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE
-        except:
+        if 'domainListGetStats' not in (dir(self.conn)):
             self.oldapi = True
 
         if self.oldapi:
@@ -367,11 +365,8 @@ class LibvirtPMDA(PMDA):
             return
 
         flags = None
-        try:
-            if not self.oldapi:
-                flags = libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE
-        except:
-            pass
+        if not self.oldapi:
+            flags = libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE
 
         if cluster == self.vm_cpustats_cluster:
             try:

Thanks,

-- 
Marko Myllynen

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