Federico Simoncelli has posted comments on this change.
Change subject: udev: Race fix- load and trigger dev rule ......................................................................
Patch Set 6: (5 inline comments)
Few small (and completely optional) enhancement. The logging part should be tested.
.................................................... File vdsm/supervdsmServer.py Line 89: Line 90: Line 91: class _SuperVdsm(object): Line 92: Line 93: UDEV_WITH_RELOAD_VERION = 181 Typo UDEV_WITH_RELOAD_VERSION Line 94: Line 95: @logDecorator Line 96: def ping(self, *args, **kwargs): Line 97: # This method exists for testing purposes
Line 90: Line 91: class _SuperVdsm(object): Line 92: Line 93: UDEV_WITH_RELOAD_VERION = 181 Line 94: We can define a log object here, probably with:
log = logging.getLogger("SuperVdsm.ServerCallback") Line 95: @logDecorator Line 96: def ping(self, *args, **kwargs): Line 97: # This method exists for testing purposes Line 98: return True
Line 234: '--property-match=DM_NAME=%s' % guid] Line 235: rc, out, err = misc.execCmd(cmd, sudo=False) Line 236: if rc: Line 237: raise OSError(errno.EINVAL, "Could not trigger change for device \ Line 238: %s, out %s\nerr %s" % (guid, out, err)) Now that we have the logger we can separate logging and exceptions:
log.error("Udevadm trigger command failed rc=%s, out="%s", err="%s"", rc, out, err) raise OSError(errno.EINVAL, "Could not trigger change for device %s" % guid) Line 239: Line 240: @logDecorator Line 241: def appropriateDevice(self, guid, thiefId): Line 242: ruleFile = _UDEV_RULE_FILE_NAME % (guid, thiefId)
Line 314: cmd = [EXT_UDEVADM, 'control', reload] Line 315: rc, out, err = misc.execCmd(cmd, sudo=False) Line 316: if rc: Line 317: raise OSError(errno.EINVAL, "Could not reload-rules for device \ Line 318: %s, out %s\nerr %s" % (guid, out, err)) Split this in logging/exception as above. Line 319: Line 320: Line 321: @utils.memoized Line 322: def __udevVersion(self):
Line 321: @utils.memoized Line 322: def __udevVersion(self): Line 323: cmd = [EXT_UDEVADM, '--version'] Line 324: rc, out, err = misc.execCmd(cmd, sudo=False) Line 325: if rc: logging for rc, out, err. Line 326: raise RuntimeError("Could not get udev version number") Line 327: return int(out) Line 328: Line 329:
-- To view, visit http://gerrit.ovirt.org/10758 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: If3b2008a3d9df2dcaf54190721c2dd9764338627 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vered Volansky vvolansk@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Lee Yarwood lyarwood@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Vered Volansky vvolansk@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server