Merge lp:~xnox/landscape-client/s390x into lp:~landscape/landscape-client/trunk

Proposed by Dimitri John Ledkov
Status: Merged
Merged at revision: 828
Proposed branch: lp:~xnox/landscape-client/s390x
Merge into: lp:~landscape/landscape-client/trunk
Diff against target: 113 lines (+92/-1)
2 files modified
landscape/monitor/processorinfo.py (+52/-1)
landscape/monitor/tests/test_processorinfo.py (+40/-0)
To merge this branch: bzr merge lp:~xnox/landscape-client/s390x
Reviewer Review Type Date Requested Status
Adam Collard (community) Approve
Alberto Donato (community) Approve
Review via email: mp+284960@code.launchpad.net

Description of the change

Add support for s390x CPU architecture

To post a comment you must log in.
Revision history for this message
Alberto Donato (ack) wrote :

Looks good, +1

A few nits inline.

Also, make lint reports a few errors:

landscape/monitor/tests/test_processorinfo.py:370:1: E302 expected 2 blank lines, found 1
landscape/monitor/tests/test_processorinfo.py:379:64: W291 trailing whitespace
landscape/monitor/tests/test_processorinfo.py:380:80: E501 line too long (89 characters)
landscape/monitor/tests/test_processorinfo.py:381:80: E501 line too long (95 characters)
landscape/monitor/tests/test_processorinfo.py:382:80: E501 line too long (90 characters)
landscape/monitor/tests/test_processorinfo.py:383:80: E501 line too long (97 characters)
landscape/monitor/tests/test_processorinfo.py:384:80: E501 line too long (94 characters)
landscape/monitor/tests/test_processorinfo.py:385:80: E501 line too long (95 characters)
landscape/monitor/tests/test_processorinfo.py:393:80: E501 line too long (84 characters)
landscape/monitor/tests/test_processorinfo.py:427:80: E501 line too long (164 characters)
landscape/monitor/tests/test_processorinfo.py:447:80: E501 line too long (164 characters)
landscape/monitor/tests/test_processorinfo.py:474:80: E501 line too long (139 characters)
landscape/monitor/processorinfo.py:278:15: E225 missing whitespace around operator
landscape/monitor/processorinfo.py:291:1: W293 blank line contains whitespace

Some are related to the sample content in the test case. I think you can drop those lines (or just cut out some of the content, since they're not actually considered by the parser.

review: Approve
Revision history for this message
Adam Collard (adam-collard) wrote :

Can we extract an (X86 equivalent) value for cache-size?

review: Needs Information
Revision history for this message
Dimitri John Ledkov (xnox) wrote :

We are targeting these two microprocessors:
https://en.wikipedia.org/wiki/IBM_z13_(microprocessor)
https://en.wikipedia.org/wiki/IBM_zEC12_(microprocessor)

L1,L2 caches are per core, L3 is shared. The L3 shared cache is not reported, but for the below z chip it would have been 64MB.

There is also storage controller per compute drawer with like extra 960MiB of L4 cache, per two chips, thus 480 MiB per core.

The cache-size reported up to landscape for x86_64 for my cpu is the shared L3 cache, for z that kind of matches the shared L3 cache as reported:
cache4 : level=3 type=Unified scope=Shared size=65536K line_size=256 associativity=16

Or do you want the largest one? Cause then it would be L4 cache (non-existant on x86_64) from the co-processing unit:
cache5 : level=4 type=Unified scope=Shared size=491520K line_size=256 associativity=30

I guess i should go for the largest one? Cause i guess ultimately cpuinfo simply reports the largest cache. Then for z13 that would be 480 MiB. I'll add that in, just to show off really how big these things are. =)

lp:~xnox/landscape-client/s390x updated
829. By Dimitri John Ledkov

pep8, review comments, and add largest cache-size.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Please re-review and merge. All comments addressed.

Revision history for this message
Adam Collard (adam-collard) :
review: Approve
lp:~xnox/landscape-client/s390x updated
830. By Dimitri John Ledkov

fix copy&paste, and try to hint string with #noqa

831. By Dimitri John Ledkov

use the right noqa

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'landscape/monitor/processorinfo.py'
--- landscape/monitor/processorinfo.py 2014-04-30 15:58:40 +0000
+++ landscape/monitor/processorinfo.py 2016-02-05 10:14:41 +0000
@@ -263,7 +263,58 @@
263 return processors263 return processors
264264
265265
266class S390XMessageFactory:
267 """Factory for s390x-based processors provides processor information.
268
269 @param source_filename: The file name of the data source.
270 """
271
272 def __init__(self, source_filename):
273 self._source_filename = source_filename
274
275 def create_message(self):
276 """Returns a list containing information about each processor."""
277 processors = []
278 vendor = None
279 cache_size = 0
280 file = open(self._source_filename)
281
282 try:
283 current = None
284
285 for line in file:
286 parts = line.split(":", 1)
287 key = parts[0].strip()
288
289 if key == "vendor_id":
290 vendor = parts[1].strip()
291 continue
292
293 if key.startswith("cache"):
294 for word in parts[1].split():
295 if word.startswith("size="):
296 cache_size = int(word[5:-1])
297 continue
298
299 if key.startswith("processor "):
300 id = int(key.split()[1])
301 model = parts[1].split()[-1]
302 current = {
303 "processor-id": id,
304 "model": model,
305 "vendor": vendor,
306 "cache-size": cache_size,
307 }
308 processors.append(current)
309 continue
310 finally:
311 file.close()
312
313 return processors
314
315
266message_factories = [("(arm*|aarch64)", ARMMessageFactory),316message_factories = [("(arm*|aarch64)", ARMMessageFactory),
267 ("ppc(64)?", PowerPCMessageFactory),317 ("ppc(64)?", PowerPCMessageFactory),
268 ("sparc[64]", SparcMessageFactory),318 ("sparc[64]", SparcMessageFactory),
269 ("i[3-7]86|x86_64", X86MessageFactory)]319 ("i[3-7]86|x86_64", X86MessageFactory),
320 ("s390x", S390XMessageFactory)]
270321
=== modified file 'landscape/monitor/tests/test_processorinfo.py'
--- landscape/monitor/tests/test_processorinfo.py 2014-05-01 14:10:46 +0000
+++ landscape/monitor/tests/test_processorinfo.py 2016-02-05 10:14:41 +0000
@@ -368,6 +368,46 @@
368 self.assertEqual(processor_1["processor-id"], 1)368 self.assertEqual(processor_1["processor-id"], 1)
369369
370370
371class S390XMessageTest(LandscapeTest):
372 """Tests for s390x message builder."""
373
374 helpers = [MonitorHelper]
375
376 S390X = """
377vendor_id : IBM/S390
378# processors : 4
379bogomips per cpu: 3033.00
380features : esan3 zarch stfle msa ldisp eimm dfp etf3eh highgprs
381cache0 : level=1 type=Data scope=Private size=128K line_size=256 associativity=8
382cache1 : level=1 type=Instruction scope=Private size=96K line_size=256 associativity=6
383cache2 : level=2 type=Data scope=Private size=2048K line_size=256 associativity=8
384cache3 : level=2 type=Instruction scope=Private size=2048K line_size=256 associativity=8
385cache4 : level=3 type=Unified scope=Shared size=65536K line_size=256 associativity=16
386cache5 : level=4 type=Unified scope=Shared size=491520K line_size=256 associativity=30
387processor 0: version = FF, identification = 018F67, machine = 2964
388processor 1: version = FF, identification = 018F67, machine = 2964
389processor 2: version = FF, identification = 018F67, machine = 2964
390processor 3: version = FF, identification = 018F67, machine = 2964
391""" # noqa
392
393 def test_read_sample_s390x_data(self):
394 """Ensure the plugin can parse /proc/cpuinfo from an IBM zSeries machine."""
395 filename = self.makeFile(self.S390X)
396 plugin = ProcessorInfo(machine_name="s390x",
397 source_filename=filename)
398 message = plugin.create_message()
399 self.assertEqual("processor-info", message["type"])
400 self.assertEqual(4, len(message["processors"]))
401
402 for id, processor in enumerate(message["processors"]):
403 self.assertEqual(
404 {"vendor": "IBM/S390",
405 "model": "2964",
406 "processor-id": id,
407 "cache-size": 491520,
408 }, processor)
409
410
371class X86MessageTest(LandscapeTest):411class X86MessageTest(LandscapeTest):
372 """Test for x86-specific message handling."""412 """Test for x86-specific message handling."""
373413

Subscribers

People subscribed via source and target branches

to all changes: