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
=== modified file 'contrib/preseeds_v2/enlist_userdata'
--- contrib/preseeds_v2/enlist_userdata 2013-04-08 19:56:51 +0000
+++ contrib/preseeds_v2/enlist_userdata 2013-04-09 21:28:22 +0000
@@ -66,7 +66,13 @@
66 import json66 import json
6767
68 def detect_ipmi():68 def detect_ipmi():
69 # TODO: Detection could be improved.69 # XXX: andreserl 2013-04-09 bug=1064527: Try to detect if node
70 # is a Virtual Machine. If it is, do not try to detect IPMI.
71 with open('/proc/cpuinfo', 'r') as cpuinfo:
72 for line in cpuinfo:
73 if line.startswith('model name') and 'QEMU' in line:
74 return (False, None)
75
70 (status, output) = commands.getstatusoutput('ipmi-locate')76 (status, output) = commands.getstatusoutput('ipmi-locate')
71 show_re = re.compile('(IPMI\ Version:) (\d\.\d)')77 show_re = re.compile('(IPMI\ Version:) (\d\.\d)')
72 res = show_re.search(output)78 res = show_re.search(output)
7379
=== modified file 'src/metadataserver/commissioning/user_data.template'
--- src/metadataserver/commissioning/user_data.template 2013-04-08 19:56:51 +0000
+++ src/metadataserver/commissioning/user_data.template 2013-04-09 21:28:22 +0000
@@ -229,7 +229,13 @@
229import time229import time
230230
231def detect_ipmi():231def detect_ipmi():
232 # TODO: Detection could be improved.232 # XXX: andreserl 2013-04-09 bug=1064527: Try to detect if node
233 # is a Virtual Machine. If it is, do not try to detect IPMI.
234 with open('/proc/cpuinfo', 'r') as cpuinfo:
235 for line in cpuinfo:
236 if line.startswith('model name') and 'QEMU' in line:
237 return (False, None)
238
233 (status, output) = commands.getstatusoutput('ipmi-locate')239 (status, output) = commands.getstatusoutput('ipmi-locate')
234 show_re = re.compile('(IPMI\ Version:) (\d\.\d)')240 show_re = re.compile('(IPMI\ Version:) (\d\.\d)')
235 res = show_re.search(output)241 res = show_re.search(output)