Juan Hernandez has posted comments on this change.

Change subject: packaging: change handling of sysctl.conf
......................................................................


Patch Set 10: (2 inline comments)

....................................................
File packaging/fedora/setup/engine-setup.py
Line 1618:     """
Line 1619:     Check and verify that the kernel.shmmax kernel parameter is 
above 35mb
Line 1620:     """
Line 1621:     # Compare to basedefs.CONST_SHMMAX
Line 1622:     cmd = [basedefs.EXEC_SYSCTL, "-b", "kernel.shmmax",]
Put this in multiple lines, just to be consistent with the style used in the 
rest of the script:

  cmd = [
      basedefs.EXEC_SYSCTL,
      "-b", "kernel.shmmax",
  ]
Line 1623:     currentShmmax, rc = utils.execCmd(cmdList=cmd, failOnError=True, 
msg=output_messages.ERR_EXP_FAILED_KERNEL_PARAMS)
Line 1624:     if currentShmmax and (int(currentShmmax) >= 
basedefs.CONST_SHMMAX):
Line 1625:         logging.debug("current shared memory max in kernel is %s, 
there is no need to update the kernel parameters", currentShmmax)
Line 1626:         return


Line 1647:     # Restart sysctl service
Line 1648:     cmd = [
Line 1649:         basedefs.EXEC_SYSTEMCTL, "restart", "systemd-sysctl.service",
Line 1650:     ]
Line 1651:     utils.execCmd(cmdList=cmd, failOnError=True, 
msg=output_messages.ERR_EXP_FAILED_KERNEL_PARAMS)
This will work in systemd based distros (Fedora, for example) but will not work 
in SysV distros (RHEL for example). Better test if the systemctl command exist 
and then use it, otherwise use sysctl, that will work in both kinds of 
distributions:

  if os.path.exists(basdefs.EXEC_SYSTEMCTL):
      cmd = [
          basedefs.EXEC_SYSTEMCTL,
          "restart",
          "systemd-sysctl.service",
       ]
   else:
       cmd = [
           basedefs.EXEC_SYSCTL,
           "-e", "-p",
        ]
Line 1652: 
Line 1653: def _addIsoDomaintoDB(uuid, description):
Line 1654:     logging.debug("Adding iso domain into DB")
Line 1655:     sqlQuery = "select inst_add_iso_storage_domain ('%s', '%s', 
'%s:%s', %s, %s)" % (uuid, description, controller.CONF["HOST_FQDN"], 
controller.CONF["NFS_MP"], 0, 0)


--
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: 10
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ohad Basan <[email protected]>
Gerrit-Reviewer: Alex Lourie <[email protected]>
Gerrit-Reviewer: Eyal Edri <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Kiril Nesenko <[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

Reply via email to