Ryan Harper has posted comments on this change.
Change subject: Add an option to create a watchdog device. ......................................................................
Patch Set 7: I would prefer that you didn't submit this
(1 inline comment)
.................................................... File vdsm/libvirtvm.py Line 2015: "Guest CPUs are reset", Line 2016: "Guest is forcably powered off", Line 2017: "Guest is requested to gracefully shutdown", Line 2018: "No action, a debug message logged") Line 2019: return actionStrings[action] I don't see libvirt python bindings that translate the action value to the readable string. So for now, we should at least reference this list of actions (0-5) on the libvirt api page in comment so we can see where they came from.
Second, I think we should guard against action value greater than 5, your indexing operation will blow up if the action isn't less than the size of your string array.
Something like, if action is not in range(0,5), raise or log an error, received unknown watchdog action and provide the value. Line 2020: self.log.debug("Watchdog event comes from guest %s(%s). " Line 2021: "Action: %s", name, vmid, actionToString(action)) Line 2022: Line 2023: def changeCD(self, drivespec):
-- To view, visit http://gerrit.ovirt.org/7535 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I2b7970a9050ab0279fe03371b9a77692fba30af8 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bing Bu Cao mars@linux.vnet.ibm.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server