Change in vdsm[master]: vdsmd and supervdsmd: extract common init tasks and add init...

asegurap at redhat.com asegurap at redhat.com
Mon Aug 12 10:38:16 UTC 2013


Antoni Segura Puimedon has posted comments on this change.

Change subject: vdsmd and supervdsmd: extract common init tasks and add init adapter
......................................................................


Patch Set 13: Code-Review-1

(4 comments)

-1 for visibility. Thanks a lot for the effort in making it easier to run on other distros!

....................................................
File vdsm/daemonAdapter
Line 19: #
Line 20: 
Line 21: import argparse
Line 22: import os
Line 23: import os.path
I don't think importing os.path is necessary if os is imported.
Line 24: import sys
Line 25: 
Line 26: from vdsm.config import config
Line 27: 


Line 58:     })
Line 59: 
Line 60:     cmd = [args.target[0]] + args.targetOptions
Line 61: 
Line 62:     if args.redirect:
I'd rather have this section use:
    sys.stdin.fileno()
    sys.stdout.fileno()
    sys.stderr.fileno()

We are redirecting stdin, stdout and stderr all to and from the same file?
I was thinking that we could allow to redirect each to different fds.

In any case, maybe we should allow O_CREAT in case they want to redirect stdout
and stderr to a file that does not yet exist.
Line 63:         # Redirction should be in last minute, otherwise it would swallow
Line 64:         # error messages.
Line 65:         os.close(0)
Line 66:         os.open(args.redirect, os.O_RDONLY)


....................................................
File vdsm/supervdsmServer.py
Line 427:         if o == "--pidfile":
Line 428:             argDict['pidfile'] = v
Line 429:         if o == "-h":
Line 430:             _usage()
Line 431:             sys.exit(0)
if an option is not any of the ones we recognize we should do:
    _usage()
    sys.exit(1)
Line 432:     if 'sockfile' not in argDict:
Line 433:         _usage()
Line 434:         sys.exit(1)
Line 435: 


....................................................
File vdsm/vdsmd.service.in
Line 7: [Service]
Line 8: Type=simple
Line 9: EnvironmentFile=-/etc/sysconfig/vdsm
Line 10: ExecStartPre=@LIBEXECDIR@/vdsmd_init_common.sh --pre-start
Line 11: ExecStart=@VDSMDIR@/daemonAdapter -c /dev/null "@VDSMDIR@/vdsm"
Systemd would put whatever goes from vdsm to the journald journal. I'm wondering if in case of crashed we might not get some useful information. What do people think about this?
Line 12: ExecStopPost=@LIBEXECDIR@/vdsmd_init_common.sh --post-stop
Line 13: Restart=always
Line 14: Nice=-20
Line 15: User=@VDSMUSER@


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id8e514df1ca88500f746242134ddb24c31588046
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Mei Liu <liumbj at linux.vnet.ibm.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list