Merge lp:~andreserl/maas/maas_detect_ipmi_bug1064527 into lp:maas/trunk

Proposed by Andres Rodriguez on 2013-04-09
Status: Merged
Approved by: Julian Edwards on 2013-04-10
Approved revision: 1462
Merged at revision: 1461
Proposed branch: lp:~andreserl/maas/maas_detect_ipmi_bug1064527
Merge into: lp:maas/trunk
Diff against target: 37 lines (+14/-2)
2 files modified
contrib/preseeds_v2/enlist_userdata (+7/-1)
src/metadataserver/commissioning/user_data.template (+7/-1)
To merge this branch: bzr merge lp:~andreserl/maas/maas_detect_ipmi_bug1064527
Reviewer Review Type Date Requested Status
Gavin Panella (community) 2013-04-09 Approve on 2013-04-09
Review via email: mp+157944@code.launchpad.net

Commit message

Improve detect_ipmi to avoid trying to detect IPMI if host is a virtual CPU

To post a comment you must log in.
Gavin Panella (allenap) wrote :

[1]

+       f = open('/proc/cpuinfo', 'r')
+       cpu_info = f.readlines()
+       f.close()
+       for line in cpu_info:
+           if line.startswith('model name') and 'QEMU' in line:
+               return (False, None)

The idiomatic way to do this in recent Python versions is:

       with open('/proc/cpuinfo', 'r') as fd:
           for line in fd:
               if line.startswith('model name') and 'QEMU' in line:
                   return (False, None)

Same for the other copy-n-pasted version of this function :-/

review: Needs Fixing
Andres Rodriguez (andreserl) wrote :

Hi Gavin,

Thanks for the review. I updated the branch to display the idiomatic as recommended.

Regards.

Gavin Panella (allenap) :
review: Approve
Julian Edwards (julian-edwards) wrote :

On 10/04/13 06:41, Gavin Panella wrote:
> Same for the other copy-n-pasted version of this function :-/

Andres, I hate to say I told you so, but.... this really needs factoring.

MAAS Lander (maas-lander) wrote :

No commit message specified.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'contrib/preseeds_v2/enlist_userdata'
2--- contrib/preseeds_v2/enlist_userdata 2013-04-08 19:56:51 +0000
3+++ contrib/preseeds_v2/enlist_userdata 2013-04-09 21:28:22 +0000
4@@ -66,7 +66,13 @@
5 import json
6
7 def detect_ipmi():
8- # TODO: Detection could be improved.
9+ # XXX: andreserl 2013-04-09 bug=1064527: Try to detect if node
10+ # is a Virtual Machine. If it is, do not try to detect IPMI.
11+ with open('/proc/cpuinfo', 'r') as cpuinfo:
12+ for line in cpuinfo:
13+ if line.startswith('model name') and 'QEMU' in line:
14+ return (False, None)
15+
16 (status, output) = commands.getstatusoutput('ipmi-locate')
17 show_re = re.compile('(IPMI\ Version:) (\d\.\d)')
18 res = show_re.search(output)
19
20=== modified file 'src/metadataserver/commissioning/user_data.template'
21--- src/metadataserver/commissioning/user_data.template 2013-04-08 19:56:51 +0000
22+++ src/metadataserver/commissioning/user_data.template 2013-04-09 21:28:22 +0000
23@@ -229,7 +229,13 @@
24 import time
25
26 def detect_ipmi():
27- # TODO: Detection could be improved.
28+ # XXX: andreserl 2013-04-09 bug=1064527: Try to detect if node
29+ # is a Virtual Machine. If it is, do not try to detect IPMI.
30+ with open('/proc/cpuinfo', 'r') as cpuinfo:
31+ for line in cpuinfo:
32+ if line.startswith('model name') and 'QEMU' in line:
33+ return (False, None)
34+
35 (status, output) = commands.getstatusoutput('ipmi-locate')
36 show_re = re.compile('(IPMI\ Version:) (\d\.\d)')
37 res = show_re.search(output)