Re: [PATCH stable-2.15 1/2] KVM: handle gracefully too old psutil versions

['Iustin Pop' via ganeti-devel]

On 21 June 2016 at 16:28, Brian Foley  wrote:

> On Sat, Jun 18, 2016 at 04:53:25AM +0200, Iustin Pop wrote:
> > On 2016-06-15 10:23:57, Brian Foley wrote:
> > > Additionally, 0.5.0 had a psutil.Process.{get,set}_cpu_affinity() API,
> > > which we use in the kvm code. In 2.0.0 API was renamed to
> cpu_affinity()
> > > and the old API deprecated, and in 3.0 the old {get,set} API was
> removed.
> > >
> > > So we currently need to restrict psutils to 2.x, but maybe it's just
> > > easier to add an __attr__ check for the new affinity API, but perhaps
> > > this should be done in a separate patch.
> >
> > Just to see if I understand: we don't support 3.x+ due to the
> > cpu_affinity API only?
>
> I think so yes. That's the only thing I saw when I looked through the
> psutil
> version history a few months ago.
>
> Thinking a little more about what I wrote above, since we require psutil
> >= 2.0
> for kvm anyway, then it would be perfectly fine to replace the use of the
> 0.5.0
> affinity API in the kvm code with the >= 2.0 API. I don't know if this on
> its
> own would be sufficient to get it to work with psutil >= 3.0, but it would
> be
> a step in the right direction. We can look at this another time though.
>

Yes, that's why I asked—it was kind of surprising to see the use of 0.5.0
API but not actually supporting 0.5.0 :)

thanks,
iustin

Re: [PATCH stable-2.15 1/2] KVM: handle gracefully too old psutil versions

['Brian Foley' via ganeti-devel]

On Sat, Jun 18, 2016 at 04:53:25AM +0200, Iustin Pop wrote:
> On 2016-06-15 10:23:57, Brian Foley wrote:
> > On Wed, Jun 15, 2016 at 07:18:38AM +0200, Iustin Pop wrote:
> > > From: Iustin Pop 
> > > 
> > > My previous pylint cleanups were done without psutil installed; as soon
> > > I installed it, pylint showed that the wheezy's version of psutil is too
> > > old (0.5.1), not having the cpu_count() function which was introduced in
> > > 2.0.0.
> > > 
> > > This change adds a simple way to detect this and to degrade gracefully
> > > by throwing an appropriate exception instead of an unclear one at
> > > runtime.
> > 
> > This is good, but we could take it a little further. First of all we
> > support any version 2.x version of psutil from 2.0.0 to 2.2.1. However
> > we probably want to warn if the user is running <2.2.0 because 2.2.0
> > fixed a number of file handle leaks.
> 
> Uh, that is hard to do properly, I don't know if many people watch
> configure-time warnings… I'll add a note to INSTALL.

Excellent. Thanks.

> > Additionally, 0.5.0 had a psutil.Process.{get,set}_cpu_affinity() API,
> > which we use in the kvm code. In 2.0.0 API was renamed to cpu_affinity()
> > and the old API deprecated, and in 3.0 the old {get,set} API was removed.
> > 
> > So we currently need to restrict psutils to 2.x, but maybe it's just
> > easier to add an __attr__ check for the new affinity API, but perhaps
> > this should be done in a separate patch.
> 
> Just to see if I understand: we don't support 3.x+ due to the
> cpu_affinity API only?

I think so yes. That's the only thing I saw when I looked through the psutil
version history a few months ago.

Thinking a little more about what I wrote above, since we require psutil >= 2.0
for kvm anyway, then it would be perfectly fine to replace the use of the 0.5.0
affinity API in the kvm code with the >= 2.0 API. I don't know if this on its
own would be sufficient to get it to work with psutil >= 3.0, but it would be
a step in the right direction. We can look at this another time though.

Cheers,
Brian.

Re: [PATCH stable-2.15 1/2] KVM: handle gracefully too old psutil versions

[Iustin Pop]

On 2016-06-15 10:23:57, Brian Foley wrote:
> On Wed, Jun 15, 2016 at 07:18:38AM +0200, Iustin Pop wrote:
> > From: Iustin Pop 
> > 
> > My previous pylint cleanups were done without psutil installed; as soon
> > I installed it, pylint showed that the wheezy's version of psutil is too
> > old (0.5.1), not having the cpu_count() function which was introduced in
> > 2.0.0.
> > 
> > This change adds a simple way to detect this and to degrade gracefully
> > by throwing an appropriate exception instead of an unclear one at
> > runtime.
> 
> This is good, but we could take it a little further. First of all we
> support any version 2.x version of psutil from 2.0.0 to 2.2.1. However
> we probably want to warn if the user is running <2.2.0 because 2.2.0
> fixed a number of file handle leaks.

Uh, that is hard to do properly, I don't know if many people watch
configure-time warnings… I'll add a note to INSTALL.

> Additionally, 0.5.0 had a psutil.Process.{get,set}_cpu_affinity() API,
> which we use in the kvm code. In 2.0.0 API was renamed to cpu_affinity()
> and the old API deprecated, and in 3.0 the old {get,set} API was removed.
> 
> So we currently need to restrict psutils to 2.x, but maybe it's just
> easier to add an __attr__ check for the new affinity API, but perhaps
> this should be done in a separate patch.

Just to see if I understand: we don't support 3.x+ due to the
cpu_affinity API only?

Patch coming shortly.

iustin

Re: [PATCH stable-2.15 1/2] KVM: handle gracefully too old psutil versions

['Brian Foley' via ganeti-devel]

On Wed, Jun 15, 2016 at 07:18:38AM +0200, Iustin Pop wrote:
> From: Iustin Pop 
> 
> My previous pylint cleanups were done without psutil installed; as soon
> I installed it, pylint showed that the wheezy's version of psutil is too
> old (0.5.1), not having the cpu_count() function which was introduced in
> 2.0.0.
> 
> This change adds a simple way to detect this and to degrade gracefully
> by throwing an appropriate exception instead of an unclear one at
> runtime.

This is good, but we could take it a little further. First of all we
support any version 2.x version of psutil from 2.0.0 to 2.2.1. However
we probably want to warn if the user is running <2.2.0 because 2.2.0
fixed a number of file handle leaks.

Additionally, 0.5.0 had a psutil.Process.{get,set}_cpu_affinity() API,
which we use in the kvm code. In 2.0.0 API was renamed to cpu_affinity()
and the old API deprecated, and in 3.0 the old {get,set} API was removed.

So we currently need to restrict psutils to 2.x, but maybe it's just
easier to add an __attr__ check for the new affinity API, but perhaps
this should be done in a separate patch.

Cheers,
Brian.

[PATCH stable-2.15 1/2] KVM: handle gracefully too old psutil versions

[Iustin Pop]

From: Iustin Pop 

My previous pylint cleanups were done without psutil installed; as soon
I installed it, pylint showed that the wheezy's version of psutil is too
old (0.5.1), not having the cpu_count() function which was introduced in
2.0.0.

This change adds a simple way to detect this and to degrade gracefully
by throwing an appropriate exception instead of an unclear one at
runtime.

Signed-off-by: Iustin Pop 
---
 lib/hypervisor/hv_kvm/__init__.py | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/lib/hypervisor/hv_kvm/__init__.py 
b/lib/hypervisor/hv_kvm/__init__.py
index 4df0246..0e5ecd0 100644
--- a/lib/hypervisor/hv_kvm/__init__.py
+++ b/lib/hypervisor/hv_kvm/__init__.py
@@ -45,13 +45,20 @@ import urllib2
 from bitarray import bitarray
 try:
   import psutil   # pylint: disable=F0401
+  psutil_err = ""
 except ImportError:
+  psutil_err = "not found"
   psutil = None
 try:
   import fdsend   # pylint: disable=F0401
 except ImportError:
   fdsend = None
 
+if psutil is not None and not hasattr(psutil, 'cpu_count'):
+  # The psutil version seems too old, we ignore it
+  psutil_err = "too old (2.0.0 needed, %s found)" % psutil.__version__
+  psutil = None
+
 from ganeti import utils
 from ganeti import constants
 from ganeti import errors
@@ -716,11 +723,14 @@ class KVMHypervisor(hv_base.BaseHypervisor):
 
 """
 if psutil is None:
-  raise errors.HypervisorError("psutil Python package not"
-   " found; cannot use CPU pinning under KVM")
+  raise errors.HypervisorError("psutil Python package %s"
+   "; cannot use CPU pinning"
+   " under KVM" % psutil_err)
 
 target_process = psutil.Process(process_id)
 if cpus == constants.CPU_PINNING_OFF:
+  # we checked this at import time
+  # pylint: disable=E1101
   target_process.set_cpu_affinity(range(psutil.cpu_count()))
 else:
   target_process.set_cpu_affinity(cpus)
-- 
2.8.1