On Tue, Sep 24, 2013 at 03:08:28PM +0200, Thomas Thrainer wrote: > On Tue, Sep 24, 2013 at 2:22 PM, Jose A. Lopes <[email protected]> wrote: > > > Add all constants in 'AutoConf' to the Hs2Py constant generation. > > > > Signed-off-by: Jose A. Lopes <[email protected]> > > --- > > src/Ganeti/HsConstants.hs | 88 > > ++++++++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 87 insertions(+), 1 deletion(-) > > > > diff --git a/src/Ganeti/HsConstants.hs b/src/Ganeti/HsConstants.hs > > index 545c30b..94e6162 100644 > > --- a/src/Ganeti/HsConstants.hs > > +++ b/src/Ganeti/HsConstants.hs > > @@ -40,7 +40,7 @@ import Data.Map (Map) > > import qualified Data.Map as Map (fromList) > > > > import AutoConf > > -import Ganeti.ConstantUtils (FrozenSet, Protocol(..)) > > +import Ganeti.ConstantUtils (FrozenSet, Protocol(..), buildVersion) > > import qualified Ganeti.ConstantUtils as ConstantUtils > > import Ganeti.Runtime (GanetiDaemon(..), MiscGroup(..), GanetiGroup(..), > > ExtraLogReason(..)) > > @@ -50,6 +50,40 @@ import qualified Ganeti.Runtime as Runtime > > import Ganeti.Types > > import qualified Ganeti.Types as Types > > > > +-- * 'autoconf' constants for Python only > > + > > +drbdBarriers :: String > > +drbdBarriers = AutoConf.drbdBarriers > > + > > +drbdNoMetaFlush :: Bool > > +drbdNoMetaFlush = AutoConf.drbdNoMetaFlush > > + > > +lvmStripecount :: Int > > +lvmStripecount = AutoConf.lvmStripecount > > + > > +-- * Various versions > > + > > +releaseVersion :: String > > +releaseVersion = AutoConf.packageVersion > > + > > +configMajor :: Int > > +configMajor = AutoConf.versionMajor > > + > > +configMinor :: Int > > +configMinor = AutoConf.versionMinor > > + > > +-- | Configuration and all protocols are supposed to remain stable > > +-- across revisions. Therefore, the revision is cleared to '0' in > > +-- configuration and protocol versions. > > +configRevision :: Int > > +configRevision = 0 > > + > > +configVersion :: Int > > +configVersion = buildVersion configMajor configMinor configRevision > > + > > +protocolVersion :: Int > > +protocolVersion = buildVersion configMajor configMinor 0 > > > > Why not use configRevision here instead of 0? Or a new constant, > protocolRevision? In any case, a comment like the one above would be nice.
The comment is actually for all 3 constants: it says 'configuration' and 'protocols'. But I will split the comment in two. The reason why there is a 'configRevision' and not a 'protocolRevision' is because in Python the constant 'CONFIG_REVISION' is actually used independently, where there is no 'PROTOCOL_REVISION'. > > > > + > > -- * Constants for 'lib/pathutils.py' > > > > osSearchPath :: [String] > > @@ -107,6 +141,20 @@ sshLoginUser = AutoConf.sshLoginUser > > sshConsoleUser :: String > > sshConsoleUser = AutoConf.sshConsoleUser > > > > +-- * 'autoconf' enable/disable > > + > > +enableConfd :: Bool > > +enableConfd = AutoConf.enableConfd > > + > > +enableMond :: Bool > > +enableMond = AutoConf.enableMond > > + > > +enableRestrictedCommands :: Bool > > +enableRestrictedCommands = AutoConf.enableRestrictedCommands > > + > > +enableSplitQuery :: Bool > > +enableSplitQuery = AutoConf.enableSplitQuery > > + > > -- * SSH constants > > > > ssh :: String > > @@ -217,6 +265,26 @@ xenKernel = AutoConf.xenKernel > > knownXenCommands :: FrozenSet String > > knownXenCommands = ConstantUtils.mkSet [xenCmdXl, xenCmdXm] > > > > +-- * KVM and socat > > + > > +kvmPath :: String > > +kvmPath = AutoConf.kvmPath > > + > > +kvmKernel :: String > > +kvmKernel = AutoConf.kvmKernel > > + > > +socatEscapeCode :: String > > +socatEscapeCode = "0x1d" > > > > I guess that's a constant coming from Python, right? Please state in the > commit message that you also converted some more Python constants, not just > the AutoConf ones. Will do. > > > > + > > +socatPath :: String > > +socatPath = AutoConf.socatPath > > + > > +socatUseCompress :: Bool > > +socatUseCompress = AutoConf.socatUseCompress > > + > > +socatUseEscape :: Bool > > +socatUseEscape = AutoConf.socatUseEscape > > + > > -- * Storage types > > > > stBlock :: String > > @@ -429,6 +497,13 @@ maxTagsPerObj = 4096 > > nodeMaxClockSkew :: Int > > nodeMaxClockSkew = 150 > > > > +-- | Disk index separator > > +diskSeparator :: String > > +diskSeparator = AutoConf.diskSeparator > > + > > +ipCommandPath :: String > > +ipCommandPath = AutoConf.ipPath > > > > Is this intentionally renamed? If so, why not in AutoConf.hs.in? Both were copied from Python: 'ipPath' was copied from 'lib/_autoconf.py' 'IP_PATH' and 'ipCommandPath' was copied from 'lib/constants.py' 'IP_COMMAND_PATH'. > > > > + > > -- * Reboot types > > > > instanceRebootSoft :: String > > @@ -1174,6 +1249,17 @@ validAllocPolicies = map Types.allocPolicyToRaw > > [minBound..] > > blockdevDriverManual :: String > > blockdevDriverManual = Types.blockDriverToRaw BlockDrvManual > > > > +-- | 'qemu-img' path, required for 'ovfconverter' > > +qemuimgPath :: String > > +qemuimgPath = AutoConf.qemuimgPath > > + > > +-- | Whether htools was enabled at compilation time > > +-- > > +-- FIXME: this should be moved next to the other enable constants, > > +-- such as, 'enableConfd', and renamed to 'enableHtools'. > > > > Why not doing that right now? Someone should look deeper at this issue because this might be a useless variable given that it does not come from 'configure'. If you look at 'Makefile.am', you'll see that this variable is set to 'True'. It makes me wonder why... > > > > +htools :: Bool > > +htools = AutoConf.htools > > + > > -- | Path generating random UUID > > randomUuidFile :: String > > randomUuidFile = ConstantUtils.randomUuidFile > > -- > > 1.8.4 > > > > > Rest LGTM, thanks. > > > -- > Thomas Thrainer | Software Engineer | [email protected] | > > Google Germany GmbH > Dienerstr. 12 > 80331 München > > Registergericht und -nummer: Hamburg, HRB 86891 > Sitz der Gesellschaft: Hamburg > Geschäftsführer: Graham Law, Christine Elizabeth Flores -- Jose Antonio Lopes Ganeti Engineering Google Germany GmbH Dienerstr. 12, 80331, München Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Graham Law, Christine Elizabeth Flores Steuernummer: 48/725/00206 Umsatzsteueridentifikationsnummer: DE813741370
