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 <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Bing Bu Cao <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: Ryan Harper <[email protected]>
Gerrit-Reviewer: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Shu Ming <[email protected]>
Gerrit-Reviewer: Xu He Jie <[email protected]>
Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
