Royce Lv has uploaded a new change for review.
Change subject: [WIP]start vdsm as subprocess in supervdsm server ......................................................................
[WIP]start vdsm as subprocess in supervdsm server
supervdsm will run as root, vdsm start needs to drop priviledge. export start vdsm function, run it as subprocess when start supervdsm server. sleep to make vdsm starts first and log ownership right.
Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Signed-off-by: Royce Lvlvroyce@linux.vnet.ibm.com --- M vdsm/supervdsmServer.py R vdsm/vdsmServer.py 2 files changed, 12 insertions(+), 31 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/91/11191/1
diff --git a/vdsm/supervdsmServer.py b/vdsm/supervdsmServer.py index 83a5803..d64076b 100755 --- a/vdsm/supervdsmServer.py +++ b/vdsm/supervdsmServer.py @@ -423,4 +423,7 @@ sys.exit(1)
if __name__ == '__main__': + vdsmProc = Process(target=startVdsm) + vdsmProc.start() + sleep(2) main() diff --git a/vdsm/vdsm b/vdsm/vdsmServer.py similarity index 77% rename from vdsm/vdsm rename to vdsm/vdsmServer.py old mode 100755 new mode 100644 index 4746a0f..337a4ca --- a/vdsm/vdsm +++ b/vdsm/vdsmServer.py @@ -10,11 +10,8 @@
import os import sys -import getopt import signal -import getpass import pwd -import grp import threading import logging import syslog @@ -99,19 +96,6 @@ log.info(str(t))
-def parse_args(): - opts, args = getopt.getopt(sys.argv[1:], "h", ["help"]) - for o, v in opts: - o = o.lower() - if o == "-h" or o == "--help": - usage() - sys.exit(0) - - if len(args) >= 1: - usage() - sys.exit(1) - - def __assertLogPermission(): if not os.access(constants.P_VDSM_LOG, os.W_OK): syslog.syslog("vdsm log directory is not accessible") @@ -127,21 +111,15 @@ sys.exit(1)
-def __assertVdsmUser(): - username = getpass.getuser() - 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) - 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) +def startVdsm(): + def dropPrivileges(): + if os.getuid() != 0: + sys.exit(1) + vdsm_uid, vdsm_gid = pwd.getpwnam(constants.VDSM_USER)[2:4:]
-if __name__ == '__main__': - __assertVdsmUser() + os.setgroups([]) + os.setgid(vdsm_gid) + os.setuid(vdsm_uid) + dropPrivileges() __assertLogPermission() - os.setpgrp() - parse_args() run()
-- To view, visit http://gerrit.ovirt.org/11191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP]start vdsm as subprocess in supervdsm server ......................................................................
Patch Set 1:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/838/ (2/2)
-- To view, visit http://gerrit.ovirt.org/11191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP]start vdsm as subprocess in supervdsm server ......................................................................
Patch Set 1:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/803/ (1/2)
-- To view, visit http://gerrit.ovirt.org/11191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP]start vdsm as subprocess in supervdsm server ......................................................................
Patch Set 1: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/803/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/838/ : FAILURE
-- To view, visit http://gerrit.ovirt.org/11191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP]start vdsm as subprocess in supervdsm server ......................................................................
Patch Set 2:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/816/ (1/2)
-- To view, visit http://gerrit.ovirt.org/11191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP]start vdsm as subprocess in supervdsm server ......................................................................
Patch Set 2:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/851/ (2/2)
-- To view, visit http://gerrit.ovirt.org/11191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP]start vdsm as subprocess in supervdsm server ......................................................................
Patch Set 2: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/816/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/851/ : FAILURE
-- To view, visit http://gerrit.ovirt.org/11191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP]start vdsm as subprocess in supervdsm server ......................................................................
Patch Set 3:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/823/ (1/2)
-- To view, visit http://gerrit.ovirt.org/11191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP]start vdsm as subprocess in supervdsm server ......................................................................
Patch Set 3:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/858/ (2/2)
-- To view, visit http://gerrit.ovirt.org/11191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP]start vdsm as subprocess in supervdsm server ......................................................................
Patch Set 3:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/823/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/858/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/11191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Yaniv Bronhaim has posted comments on this change.
Change subject: [WIP]start vdsm as subprocess in supervdsm server ......................................................................
Patch Set 3: I would prefer that you didn't submit this
(8 inline comments)
did you try to run it?
.................................................... File Makefile.am Line 64: PEP8_BLACKLIST = config.py,constants.py Line 65: Line 66: check-local: Line 67: find . -path './.git' -prune -type f -o \ Line 68: -name '*.py' ! 'vdsmServer.py' -o \ why?? Line 69: -name '*.py.in' | xargs $(PYFLAKES) Line 70: $(PEP8) --exclude="$(PEP8_BLACKLIST)" --filename '*.py,*.py.in' \ Line 71: $(PEP8_WHITELIST) Line 72: @if test -f .gitignore; then \
.................................................... File tests/Makefile.am Line 49 Line 50 Line 51 Line 52 Line 53 you can write in the comment that you remove the tests until we'll fix them, but i prefer that this series of patches will also include superVdsmTests .
.................................................... File vdsm.spec.in Line 635: %{_datadir}/%{vdsm_name}/supervdsm.py* Line 636: %{_datadir}/%{vdsm_name}/supervdsmServer.py* Line 637: %{_datadir}/%{vdsm_name}/vmChannels.py* Line 638: %{_datadir}/%{vdsm_name}/tc.py* Line 639: %{_datadir}/%{vdsm_name}/vdsmServer.py* don't you need to make supervdsmServer.py python file executable? Line 640: %{_datadir}/%{vdsm_name}/vdsm-restore-net-config Line 641: %{_datadir}/%{vdsm_name}/vdsm-store-net-config Line 642: %{_datadir}/%{vdsm_name}/vm.py* Line 643: %{_datadir}/%{vdsm_name}/write-net-config
.................................................... File vdsm/supervdsmServer.py Line 423: sys.exit(1) Line 424: Line 425: if __name__ == '__main__': Line 426: vdsmProc = Process(target=startVdsm) Line 427: vdsmProc.start() import startVdsm is missing..
what if startVdsm gets to log creation after supervdsmServer creates the file as root?
maybe log initialization should be here with root permissions if not exist with chown to vdsm user, if exist we can move forward Line 428: sleep(2)
.................................................... File vdsm/vdsmd.init.in Line 22: ### END INIT INFO Line 23: Line 24: . @LIBEXECDIR@/ovirt_functions.sh Line 25: Line 26: SVDSM_BIN=@VDSMDIR@/supervdsmServer you didn't remove the py suffix... Line 27: CONF_FILE=@CONFDIR@/vdsm.conf Line 28: GETCONFITEM=@VDSMDIR@/get-conf-item Line 29: prog=vdsm Line 30: PIDFILE=@VDSMRUNDIR@/vdsmd.pid
.................................................... File vdsm/vdsmServer.py Line 1: #!/usr/bin/python #!/usr/bin/python is redundant if you want to omit the option to run this file and only use it as package. Line 2: # Line 3: # Copyright 2009 Red Hat, Inc. and/or its affiliates. Line 4: # Line 5: # Licensed to you under the GNU General Public License as published by
Line 27: pthreading.monkey_patch() Line 28: Line 29: loggerConfFile = constants.P_VDSM_CONF + 'logger.conf' Line 30: Line 31: I guess you wont use this function as well if you remove parse_args Line 32: def usage(): Line 33: print "Usage: vdsm [OPTIONS]" Line 34: print " -h - Display this help message" Line 35:
Line 120: os.setgroups([]) Line 121: os.setgid(vdsm_gid) Line 122: os.setuid(vdsm_uid) Line 123: dropPrivileges() Line 124: __assertLogPermission() why did you omit os.setpgrp(). don't we need it?
-- To view, visit http://gerrit.ovirt.org/11191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Royce Lv has posted comments on this change.
Change subject: [WIP]start vdsm as subprocess in supervdsm server ......................................................................
Patch Set 3: (7 inline comments)
Yes, Tested it. Most of the problem may due to disorder rebase problem:(I'll clean it soon.
.................................................... File Makefile.am Line 64: PEP8_BLACKLIST = config.py,constants.py Line 65: Line 66: check-local: Line 67: find . -path './.git' -prune -type f -o \ Line 68: -name '*.py' ! 'vdsmServer.py' -o \ because vdsm has "import vdsmDebugPlugin" but not used it, so pyflakes will shout. Line 69: -name '*.py.in' | xargs $(PYFLAKES) Line 70: $(PEP8) --exclude="$(PEP8_BLACKLIST)" --filename '*.py,*.py.in' \ Line 71: $(PEP8_WHITELIST) Line 72: @if test -f .gitignore; then \
.................................................... File tests/Makefile.am Line 49 Line 50 Line 51 Line 52 Line 53 OK
.................................................... File vdsm.spec.in Line 635: %{_datadir}/%{vdsm_name}/supervdsm.py* Line 636: %{_datadir}/%{vdsm_name}/supervdsmServer.py* Line 637: %{_datadir}/%{vdsm_name}/vmChannels.py* Line 638: %{_datadir}/%{vdsm_name}/tc.py* Line 639: %{_datadir}/%{vdsm_name}/vdsmServer.py* it's not the tail of this branch, checkout the tail and you'll see it is executable.This one is mainly for vdsm refactor. Line 640: %{_datadir}/%{vdsm_name}/vdsm-restore-net-config Line 641: %{_datadir}/%{vdsm_name}/vdsm-store-net-config Line 642: %{_datadir}/%{vdsm_name}/vm.py* Line 643: %{_datadir}/%{vdsm_name}/write-net-config
.................................................... File vdsm/supervdsmServer.py Line 423: sys.exit(1) Line 424: Line 425: if __name__ == '__main__': Line 426: vdsmProc = Process(target=startVdsm) Line 427: vdsmProc.start() Sorry, I need to re-orgnize my patch order, the import seems been rebased into other patch.
The log is indeed a issue need to be handled first. Line 428: sleep(2)
.................................................... File vdsm/vdsmd.init.in Line 22: ### END INIT INFO Line 23: Line 24: . @LIBEXECDIR@/ovirt_functions.sh Line 25: Line 26: SVDSM_BIN=@VDSMDIR@/supervdsmServer Wrong rebase again, sorry, the rename is in another patch:( Line 27: CONF_FILE=@CONFDIR@/vdsm.conf Line 28: GETCONFITEM=@VDSMDIR@/get-conf-item Line 29: prog=vdsm Line 30: PIDFILE=@VDSMRUNDIR@/vdsmd.pid
.................................................... File vdsm/vdsmServer.py Line 1: #!/usr/bin/python Done Line 2: # Line 3: # Copyright 2009 Red Hat, Inc. and/or its affiliates. Line 4: # Line 5: # Licensed to you under the GNU General Public License as published by
Line 27: pthreading.monkey_patch() Line 28: Line 29: loggerConfFile = constants.P_VDSM_CONF + 'logger.conf' Line 30: Line 31: Done Line 32: def usage(): Line 33: print "Usage: vdsm [OPTIONS]" Line 34: print " -h - Display this help message" Line 35:
-- To view, visit http://gerrit.ovirt.org/11191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP]export a priviledged vdsm startup for supervdsm server ......................................................................
Patch Set 4:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/865/ (1/3)
-- To view, visit http://gerrit.ovirt.org/11191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP]export a priviledged vdsm startup for supervdsm server ......................................................................
Patch Set 4:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/8/ (2/3)
-- To view, visit http://gerrit.ovirt.org/11191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP]export a priviledged vdsm startup for supervdsm server ......................................................................
Patch Set 4:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/900/ (3/3)
-- To view, visit http://gerrit.ovirt.org/11191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP]export a priviledged vdsm startup for supervdsm server ......................................................................
Patch Set 4: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/865/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/900/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/8/ : FAILURE
-- To view, visit http://gerrit.ovirt.org/11191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I540b1d3f3c823433f100f4803f31322fc7ee2153 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Yaniv Bronhaim has posted comments on this change.
Change subject: [WIP]export a priviledged vdsm startup for supervdsm server ......................................................................
Patch Set 4:
Not relevant any more due to http://gerrit.ovirt.org/#/c/11051/. Please abandon
Itamar Heim has posted comments on this change.
Change subject: [WIP]export a priviledged vdsm startup for supervdsm server ......................................................................
Patch Set 4:
ping?
Yaniv Bronhaim has posted comments on this change.
Change subject: [WIP]export a priviledged vdsm startup for supervdsm server ......................................................................
Patch Set 4: Code-Review-1
all such wip can be removed. it was old design that Royce and I worked on. currently (ovirt-3.3 and above) we've started to implement the usage of supervdsm process as external service that vdsm communicate with
Itamar Heim has abandoned this change.
Change subject: [WIP]export a priviledged vdsm startup for supervdsm server ......................................................................
Abandoned
please re-open if needed
vdsm-patches@lists.fedorahosted.org