Juan Hernandez has posted comments on this change.
Change subject: packaging: change handling of sysctl.conf
......................................................................
Patch Set 7: (6 inline comments)
....................................................
File packaging/fedora/setup/basedefs.py
Line 139: EXEC_SYSCTL="/sbin/sysctl"
Line 140: EXEC_SETSEBOOL="/usr/sbin/setsebool"
Line 141: EXEC_SEMANAGE="/usr/sbin/semanage"
Line 142: EXEC_KEYTOOL="/usr/bin/keytool"
Line 143: EXEC_REMOVE="/bin/rm"
This is not needed, you can use the Python builtin function os.remove(...).
Line 144:
Line 145: CONST_BASE_MAC_ADDR="00:1A:4A"
Line 146: CONST_DEFAULT_MAC_RANGE="00:1a:4a:16:84:02-00:1a:4a:16:84:fd"
Line 147: CONST_MINIMUM_SPACE_ISODOMAIN=350
....................................................
File packaging/fedora/setup/engine-cleanup.py
Line 57: MSG_INFO_CLEANUP_OK = "\nCleanup finished successfully!"
Line 58: MSG_INFO_FINISHED_WITH_ERRORS = "\nCleanup finished with errors,
please see log file"
Line 59: MSG_INFO_STOP_INSTALL_EXIT = "Cleanup stopped, Goodbye."
Line 60: MSG_INFO_KILL_DB_CONNS="Stopping all connections to DB"
Line 61: MSG_INFO_DELETE_SYSCTL_CONF="Error deleting %s"
%basedefs.FILE_ENGINE_SYSCTL
Whitespace around the = and after th %?
Line 62:
Line 63: MSG_ALERT_CLEANUP = "WARNING: Executing %s cleanup utility.\n\
Line 64: This utility will wipe all existing data including configuration
settings, certificates and database.\n\
Line 65: In addition, all existing DB connections will be closed." % (PROD_NAME)
Line 368: return True
Line 369:
Line 370: def deleteSysctlConf():
Line 371: cmd=[basedefs.EXEC_REMOVE, basedefs.FILE_ENGINE_SYSCTL]
Line 372: output, rc = utils.execCmd(cmdList=cmd, failOnError=True,
msg=MSG_INFO_DELETE_SYSCTL_CONF)
If you use os.remove(...) this function might not be needed.
Line 373:
Line 374: def stopEngine():
Line 375: logging.debug("stoping %s service." %
basedefs.ENGINE_SERVICE_NAME)
Line 376:
Line 432: runFunc([db.backup, db.drop, cleanPgpass], MSG_INFO_REMOVE_DB)
Line 433:
Line 434: # Remove 00-ovirt.conf
Line 435: if os.path.exists(basedefs.FILE_ENGINE_SYSCTL):
Line 436: deleteSysctlConf()
You can use os.remove(...) instead of that function.
Line 437:
Line 438: # Remove CA
Line 439: if ca.exists() and options.remove_ca:
Line 440: runFunc([ca.backup, ca.remove], MSG_INFO_REMOVE_CA)
....................................................
File packaging/fedora/setup/engine-setup.py
Line 1627: # Decide what is the file that we are going to modify according
to the
Line 1628: # existence or not of sysctl.conf, it exists in older
distributions but
Line 1629: # has been replaced by a sysctl.d directory in newer
distributions using
Line 1630: # systemd
Line 1631:
Please remove this ^ empty line.
Line 1632: sysctlFile = basedefs.FILE_SYSCTL
Line 1633: if not os.path.exists(sysctlFile):
Line 1634: sysctlFile = basedefs.FILE_ENGINE_SYSCTL
Line 1635:
Line 1634: sysctlFile = basedefs.FILE_ENGINE_SYSCTL
Line 1635:
Line 1636: # Create the chosen file if it doesn't exist.
Line 1637: if not os.path.exists(sysctlFile):
Line 1638: open(sysctlFile, "w").close()
These ^ two lines need to have less indentation, they are not part of the
previous if.
Line 1639:
Line 1640: # If we got here, it means we need to update kernel.shmmax in
sysctl.conf
Line 1641: txtHandler = utils.TextConfigFileHandler(sysctlFile)
Line 1642: txtHandler.open()
--
To view, visit http://gerrit.ovirt.org/8705
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ca4988beea1627b2da442a7fcf8fef3cf3db6a5
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ohad Basan <[email protected]>
Gerrit-Reviewer: Alex Lourie <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Moran Goldboim <[email protected]>
Gerrit-Reviewer: Ofer Schreiber <[email protected]>
Gerrit-Reviewer: Ohad Basan <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
