Change in vdsm[master]: utils: Add systemd_run command modifier

nsoffer at redhat.com nsoffer at redhat.com
Mon Apr 27 17:17:45 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: utils: Add systemd_run command modifier
......................................................................


Patch Set 1:

(3 comments)

https://gerrit.ovirt.org/#/c/40239/1/configure.ac
File configure.ac:

Line 327: AC_PATH_PROG([SUDO_PATH], [sudo], [/usr/bin/sudo])
Line 328: AC_PATH_PROG([SU_PATH], [su], [/bin/su])
Line 329: AC_PATH_PROG([SYSCTL_PATH], [sysctl], [/sbin/sysctl])
Line 330: AC_PATH_PROG([SYSTEMCTL_PATH], [systemctl], [/bin/systemctl])
Line 331: AC_PATH_PROG([SYSTEMD_RUN_PATH], [systemd-run], [/usr/bin/systemd-run])
> if only one place uses it - define it there with CommandPath
We use it in two places (sudoers and cmdutils.py)
Line 332: AC_PATH_PROG([TAR_PATH], [tar], [/bin/tar])
Line 333: AC_PATH_PROG([TC_PATH], [tc], [/sbin/tc])
Line 334: AC_PATH_PROG([TEE_PATH], [tee], [/usr/bin/tee])
Line 335: AC_PATH_PROG([TOUCH_PATH], [touch], [/bin/touch])


https://gerrit.ovirt.org/#/c/40239/1/lib/vdsm/cmdutils.py
File lib/vdsm/cmdutils.py:

Line 52:     command.extend(cmd)
Line 53:     return command
Line 54: 
Line 55: 
Line 56: def systemd_run(cmd, scope=False, unit=None, slice=None):
> this is specific code for systemd - shouldn't be in vdsm core. either have 
vdsm-tool is a command line tool - it does not make sense to start another python process to do this.

We can create a compatibility layer in vdsm library, so it can be used in both vdsm daemon and the tool - but the question is what should we use when systemd is not available - cgexec?

So this should be abstract command, something like run_as_service?
Line 57:     command = [constants.EXT_SYSTEMD_RUN]
Line 58:     if scope:
Line 59:         command.append('--scope')
Line 60:     if unit:


https://gerrit.ovirt.org/#/c/40239/1/vdsm/sudoers.vdsm.in
File vdsm/sudoers.vdsm.in:

Line 12:     @MV_PATH@ /etc/multipath.conf *, \
Line 13:     @CP_PATH@ * /etc/iscsi/iscsid.conf, \
Line 14:     @SERVICE_PATH@ iscsid *, \
Line 15:     @BINDIR@/vdsm-tool service-restart multipathd, \
Line 16:     @BINDIR@/vdsm-tool service-reload multipathd, \
> if adding any permission I would add vdsm-tool call for it which hides the 
What do you mean by adding any permission?

Anyway, I think we should drop this and run this in supervdsm instead of sudo, but this requires rather big change - running mount from supervdsm, and I don't want to mix it with this fix.
Line 17:     @ISCSIADM_PATH@ *, \
Line 18:     @LVM_PATH@, \
Line 19:     @CAT_PATH@ /sys/block/*/device/../../*, \
Line 20:     @CAT_PATH@ /sys/devices/platform/host*, \


-- 
To view, visit https://gerrit.ovirt.org/40239
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic35bdae752228b6716c25f0b9975fc500897a592
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list