Hello Adam Litke, Dan Kenigsberg, Ohad Basan,

I'd like you to do a code review. Please visit

http://gerrit.ovirt.org/10313

to review the following change.

Change subject: Check if log file exist on startup and log is accessible ......................................................................

Check if log file exist on startup and log is accessible

If file isn't exist we need to continue. Otherwise, os.access returns false and we report permission error. When continue we need to verify that vdsm has the right permissions to create the log file in the log directory.

this patch also add usage in syslog instead of assert report

Change-Id: I05f4eae90c9388302dc316af41bba7256cecacf7 Bug-Id: https://bugzilla.redhat.com/show_bug.cgi?id=870139 Signed-off-by: Yaniv Bronhaim ybronhei@redhat.com Reviewed-on: http://gerrit.ovirt.org/10194 Reviewed-on: http://gerrit.ovirt.org/9700 Reviewed-by: Dan Kenigsberg danken@redhat.com Reviewed-by: Ohad Basan obasan@redhat.com Reviewed-by: Adam Litke agl@us.ibm.com Tested-by: Adam Litke agl@us.ibm.com --- M vdsm/vdsm 1 file changed, 25 insertions(+), 7 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/13/10313/1

diff --git a/vdsm/vdsm b/vdsm/vdsm index 8df24fb..4746a0f 100755 --- a/vdsm/vdsm +++ b/vdsm/vdsm @@ -17,6 +17,7 @@ import grp import threading import logging +import syslog from logging import config as lconfig

from vdsm import constants @@ -111,19 +112,36 @@ sys.exit(1)

+def __assertLogPermission(): + if not os.access(constants.P_VDSM_LOG, os.W_OK): + syslog.syslog("vdsm log directory is not accessible") + sys.exit(1) + + logfile = constants.P_VDSM_LOG + "/vdsm.log" + if not os.path.exists(logfile): + # if file not exist, and vdsm has an access to log directory- continue + return + + if not os.access(logfile, os.W_OK): + syslog.syslog("error in accessing vdsm log file") + sys.exit(1) + + def __assertVdsmUser(): username = getpass.getuser() - assert username == constants.VDSM_USER, ( - "VDSM failed to start: running user is not %s, trying " - "to run from user %s" % (constants.VDSM_USER, username) - ) + if username != constants.VDSM_USER: + syslog.syslog("VDSM failed to start: running user is not %s, trying " + "to run from user %s" % (constants.VDSM_USER, username)) + sys.exit(1) group = grp.getgrnam(constants.VDSM_GROUP) - assert (constants.VDSM_USER in group.gr_mem or - pwd.getpwnam(constants.VDSM_USER).pw_gid == group.gr_gid), \ - "VDSM failed to start: vdsm user is not in KVM group" + if (constants.VDSM_USER not in group.gr_mem) and \ + (pwd.getpwnam(constants.VDSM_USER).pw_gid != group.gr_gid): + syslog.syslog("VDSM failed to start: vdsm user is not in KVM group") + sys.exit(1)

if __name__ == '__main__': __assertVdsmUser() + __assertLogPermission() os.setpgrp() parse_args() run()

-- To view, visit http://gerrit.ovirt.org/10313 To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange Gerrit-Change-Id: I05f4eae90c9388302dc316af41bba7256cecacf7 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.2 Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Ohad Basan obasan@redhat.com