Dan Kenigsberg has posted comments on this change. Change subject: Alert to syslog if vdsm log has wrong user\group permissions ......................................................................
Patch Set 4: I would prefer that you didn't submit this (4 inline comments) .................................................... File vdsm/vdsm Line 1: #!/usr/bin/python Line 2: # Line 3: # Copyright 2009 Red Hat, Inc. and/or its affiliates. Could you keep the changes to this file in a separate patch? after all, they are required only due to a broken 'su'. On their on patch, they are easier to revert when the su issue is solved. Line 4: # Line 5: # Licensed to you under the GNU General Public License as published by Line 6: # the Free Software Foundation; either version 2 of the License, or Line 7: # (at your option) any later version. See the files README and Line 117: Line 118: def __assertLogPermission(): Line 119: logfile = constants.P_VDSM_LOG + "vdsm.log" Line 120: try: Line 121: with open(logfile, 'w'): 'w' zaps whatever was on the file. user 'a'. or better: os.access. Line 122: pass Line 123: except IOError: Line 124: msg = "error in log file permissions: %s" % logfile Line 125: syslog(msg) Line 130: username = getpass.getuser() Line 131: if username != constants.VDSM_USER: Line 132: msg = "VDSM failed to start: running user is not %s, trying " \ Line 133: "to run from user %s" % (constants.VDSM_USER, username) Line 134: syslog(msg) why is it any better than assert? (by vdsm default asserts go to syslog, too) Line 135: sys.exit(1) Line 136: group = grp.getgrnam(constants.VDSM_GROUP) Line 137: if constants.VDSM_USER not in group.gr_mem and \ Line 138: pwd.getpwnam(constants.VDSM_USER).pw_gid != group.gr_gid: .................................................... File vdsm/vdsmd.init.in Line 506: test_conflicting_conf); then Line 507: return 1 Line 508: fi Line 509: Line 510: if ! verify_log_permissions; then well, a failure may mean other stuff, such as a missing directory, or an IO error. Line 511: log_failure_msg "Permissions error with vdsm log file" Line 512: return 1 Line 513: fi Line 514: -- To view, visit http://gerrit.ovirt.org/9118 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8bd379803b01627d6897992ee8798c6a22195b0f Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Shu Ming <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
