Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Pcp dmcache fix #74

Merged
merged 2 commits into from Feb 22, 2016

Conversation

Projects
None yet
2 participants
Contributor

pcuzner commented Feb 22, 2016

small patch making the lvname column width dynamic, so names aren't truncated. This truncation was becoming more of a problem with thinpools, since adding a cache to a thinpool has been introduced which adds _tdata to the end of the LV name

Contributor

natoscott commented Feb 22, 2016

Hi Paul,

Thanks! I'm seeing a regression test failure when I run this new code through the tests (qa/829 shell script in the pcp git tree in particular, which uses the qa/archives/dm-io PCP archive) ...

Traceback (most recent call last):
File "/usr/libexec/pcp/bin/pcp-dmcache", line 169, in
manager.run()
File "/usr/lib64/python2.7/site-packages/pcp/pmcc.py", line 623, in run
self._printer.report(self)
File "/usr/libexec/pcp/bin/pcp-dmcache", line 154, in report
self.report_values(group, width=max_lv)
File "/usr/libexec/pcp/bin/pcp-dmcache", line 122, in report_values
vgname, lvname = re.split(r'(?<=\w)-(?=\w)',name)
ValueError: need more than 1 value to unpack

This archive has volume names like "dmcache0" (containing no hyphen) so I think that's causing the re.split line to choke. Looking closer, the results of the re.split appear to be unused - is that leftover code from earlier naming experiments perhaps? (can that, and the re module import, be safely removed? seems that way - if I take those out, and tweak the expected test output slightly to match the intent of your changes, all appears to be functional). Could you just confirm that re use was not needed in the end? Thanks!

cheers.

@natoscott natoscott merged commit efbb5b9 into performancecopilot:master Feb 22, 2016

Contributor

pcuzner commented Feb 22, 2016

interesting. so you have a system that doesn't pass in the <vgname>-<lvname> syntax?

Contributor

natoscott commented Feb 22, 2016

| Could you just confirm that re use was not needed

Pretty sure this is the case, so I've tidied up & merged. If needed, could you post a followup commit Paul? Thanks again!

Contributor

natoscott commented Feb 22, 2016

| interesting. so you have a system that doesn't pass in the <vgname>-<lvname> syntax?

At the time that test archive was created, yep. I no longer have that machine though, so not sure which kernel/dm versions were involved back then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment