Juan Hernandez has posted comments on this change.
Change subject: packaging: change handling of sysctl.conf
......................................................................
Patch Set 4: (6 inline comments)
....................................................
File packaging/fedora/setup/basedefs.py
Line 107:
Line 108: #Locations of kernel configuration files
Line 109: FILE_SYSCTL="/etc/sysctl.conf"
Line 110: DIR_SYSCTL="/etc/sysctl.d"
Line 111: FILE_ENGINE_SYSCTL = os.path.join(DIR_SYSCTL, "00-ovirt-engine.conf")
Put spaces around the "=" in FILE_SYSCTL and DIR_SYSCTL as well.
Line 112:
Line 113: # ISO
Line 114: ISO_DISPLAY_NAME = "ISO_DOMAIN"
Line 115: DEFAULT_ISO_EXPORT_PATH = "/var/lib/exports/iso"
....................................................
File packaging/fedora/setup/engine-setup.py
Line 1623: if currentShmmax and (int(currentShmmax) >=
basedefs.CONST_SHMMAX):
Line 1624: logging.debug("current shared memory max in kernel is %s,
there is no need to update the kernel parameters", currentShmmax)
Line 1625: return
Line 1626:
Line 1627: # In some systems the sysctl file doesn't exist, so we may need
to create it:
This line ^ doesn't belong here, please remove it.
Line 1628: # Decide what is the file that we are going to modify according
to the
Line 1629: # existence or not of sysctl.conf, it exists in older
distributions but
Line 1630: # has been replaced by a sysctl.d directory in newer
distributions using
Line 1631: # systemd
Line 1630: # has been replaced by a sysctl.d directory in newer
distributions using
Line 1631: # systemd
Line 1632:
Line 1633: sysctlFile = basedefs.FILE_SYSCTL
Line 1634: if not os.path.exists(basedefs.sysctlFile):
Here ^ you should use just "sysctlFile", not "basedefs.sysctlFile".
Line 1635: sysctlFile = basedefs.FILE_ENGINE_SYSCTL
Line 1636: # Create the chosen file if it doesn't exist.
Line 1637: open(basedefs.sysctlFile, "w").close()
Line 1638:
Line 1631: # systemd
Line 1632:
Line 1633: sysctlFile = basedefs.FILE_SYSCTL
Line 1634: if not os.path.exists(basedefs.sysctlFile):
Line 1635: sysctlFile = basedefs.FILE_ENGINE_SYSCTL
Add a empty line here, it makes things clearer.
Line 1636: # Create the chosen file if it doesn't exist.
Line 1637: open(basedefs.sysctlFile, "w").close()
Line 1638:
Line 1639: # If we got here, it means we need to update kernel.shmmax in
sysctl.conf
Line 1632:
Line 1633: sysctlFile = basedefs.FILE_SYSCTL
Line 1634: if not os.path.exists(basedefs.sysctlFile):
Line 1635: sysctlFile = basedefs.FILE_ENGINE_SYSCTL
Line 1636: # Create the chosen file if it doesn't exist.
I am missing the "if" to check if that file exists or not.
Line 1637: open(basedefs.sysctlFile, "w").close()
Line 1638:
Line 1639: # If we got here, it means we need to update kernel.shmmax in
sysctl.conf
Line 1640: txtHandler = utils.TextConfigFileHandler(sysctlFile)
Line 1633: sysctlFile = basedefs.FILE_SYSCTL
Line 1634: if not os.path.exists(basedefs.sysctlFile):
Line 1635: sysctlFile = basedefs.FILE_ENGINE_SYSCTL
Line 1636: # Create the chosen file if it doesn't exist.
Line 1637: open(basedefs.sysctlFile, "w").close()
Just "sysctlFile", not "basedefs.sysctlFile".
Line 1638:
Line 1639: # If we got here, it means we need to update kernel.shmmax in
sysctl.conf
Line 1640: txtHandler = utils.TextConfigFileHandler(sysctlFile)
Line 1641: 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: 4
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
