Initial version of pcied #60
Conversation
|
This pull request introduces 1 alert when merging b21b15e into c617191 - view on LGTM.com new alerts:
|
|
This pull request introduces 7 alerts when merging e059bb3 into c617191 - view on LGTM.com new alerts:
|
calling `pcieutil pcie-check`.
|
This pull request introduces 1 alert when merging df98786 into c617191 - view on LGTM.com new alerts:
|
|
This pull request introduces 2 alerts when merging 5aee4c2 into c617191 - view on LGTM.com new alerts:
|
sonic-pcied/scripts/pcied
Outdated
|
|
||
| tmp_pcie_status_f="/tmp/pcie_status_f" | ||
| os.system('sudo pcieutil pcie-check | grep "%s" > %s'%(PCIE_RESULT_REGIX, tmp_pcie_status_f)) | ||
| f=open(tmp_pcie_status_f, "r") |
There was a problem hiding this comment.
Instead of using /tmp/ file to query results. We should use STATE_DB as a place-holder.
sonic-pcied/scripts/pcied
Outdated
|
|
||
| def check_pcie_devices(self): | ||
| platform, hwsku = get_platform_and_hwsku() | ||
| pciefilePath = "/".join([PLATFORM_ROOT_PATH, platform, "plugins", PCIE_CONF_FILE]) |
There was a problem hiding this comment.
is PCIE_CONF_FILE a plugin or a config file? If it's not a plugin I would suggest putting it under platform folder.
There was a problem hiding this comment.
Good point, I'm planing to work on additional feature on sonic-pcie again. I will fix this with that.
sonic-pcied/scripts/pcied
Outdated
| sys.path.append(os.path.abspath(pciefilePath)) | ||
| if not os.path.exists(pciefilePath): | ||
| logger.log_error("Platform pcie configuration file doesn't exist! exit pcied") | ||
| sys.exit("Platform PCIe Configuration file doesn't exist!") |
There was a problem hiding this comment.
I would suggest moving this check outside this function and do it before entering the loop, or it will periodically print out error msg on the platform which doesn't have this supported?
sonic-pcied/scripts/pcied
Outdated
| pcie_status = f.readline() | ||
| if "PASSED" in pcie_status: | ||
| self.update_state_db("PCIE_STATUS|", "PCIE_DEVICES", "PASSED") | ||
| logger.log_info("PCIe device status check : PASSED") |
There was a problem hiding this comment.
do we want it to be printed out each 60s?
There was a problem hiding this comment.
I thought it's ok to indicate the pcie status every 60 seconds. Do you have any other suggestion? I was also thinking about logging only when its status changes.
There was a problem hiding this comment.
IMO logging only on when status change is better.
|
This pull request introduces 1 alert when merging e43ca20 into c530587 - view on LGTM.com new alerts:
|
| self.state_db.connect("STATE_DB") | ||
|
|
||
| def check_pcie_devices(self): | ||
| cmd = [ 'sudo', 'pcieutil', 'pcie-check' ] |
There was a problem hiding this comment.
pcieutil is absent in pmon container. Please view on these links: sonic-net/sonic-buildimage#5236
pcied will run in pmon container to monitor the pcie device status during run time.
It will update the state db with current pcie status so other monitoring service can read to alert any failure status.