Change in vdsm[master]: vdsm-tool: add node register tool

danken at redhat.com danken at redhat.com
Fri Dec 20 12:40:17 UTC 2013


Dan Kenigsberg has posted comments on this change.

Change subject: vdsm-tool: add node register tool
......................................................................


Patch Set 10: Code-Review-1

(5 comments)

....................................................
Commit Message
Line 6: 
Line 7: vdsm-tool: add node register tool
Line 8: 
Line 9: register.py is a tool to register a node into engine.
Line 10: New new verb 'register' does a http request to register a node into engine.
New new - > The new.
Line 11: 
Line 12: Change-Id: I65b508d2a365108950c0758134a6d9c024d04f2c


....................................................
File lib/vdsm/tool/Makefile.am
Line 38: 	__init__.py \
Line 39: 	dummybr.py \
Line 40: 	nwfilter.py \
Line 41: 	configurator.py \
Line 42: 	register.py \
let's not worsen this list's alphabetization.
Line 43: 	passwd.py \
Line 44: 	restore_nets.py \
Line 45: 	seboolsetup.py \
Line 46: 	service.py \


....................................................
File lib/vdsm/tool/register.py
Line 50: 
Line 51: from vdsm.ipwrapper import routeGet, Route
Line 52: from vdsm.utils import getHostUUID
Line 53: 
Line 54: _LOG = logging.getLogger(__file__)
Getting a logger before configuring logging module seldom ends well.
Line 55: 
Line 56: 
Line 57: def _silent_restorecon(path):
Line 58:     """Execute selinux restorecon cmd to determined file


Line 347: 
Line 348:     if args.verbose and not args.enable_log:
Line 349:         raise RuntimeError("Verbose mode requires --enable-log")
Line 350: 
Line 351:     if args.enable_log:
why keep --enable-log?

I think you should always configure logging to use sys.stderr (not stdout).

With no --berbose, the error level should be ERROR.

Having --verbose should reduce it to DEBUG.
Line 352:         log_file = "{0}/{1}-{2}.log".format(
Line 353:             "/var/log/vdsm",
Line 354:             datetime.datetime.now().strftime("%Y-%b-%d_%H-%M-%S"),
Line 355:             "ovirt-node-registration"


....................................................
File vdsm.spec.in
Line 1092: %{python_sitearch}/%{vdsm_name}/tool/dummybr.py*
Line 1093: %{python_sitearch}/%{vdsm_name}/tool/nwfilter.py*
Line 1094: %{python_sitearch}/%{vdsm_name}/tool/configurator.py*
Line 1095: %{_libexecdir}/%{vdsm_name}/libvirt_configure.sh
Line 1096: %{python_sitearch}/%{vdsm_name}/tool/register.py*
keep in order, please.
Line 1097: %{python_sitearch}/%{vdsm_name}/tool/passwd.py*
Line 1098: %{python_sitearch}/%{vdsm_name}/tool/restore_nets.py*
Line 1099: %{python_sitearch}/%{vdsm_name}/tool/seboolsetup.py*
Line 1100: %{python_sitearch}/%{vdsm_name}/tool/service.py*


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I65b508d2a365108950c0758134a6d9c024d04f2c
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller at redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at fedoraproject.org>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list