On Tue, Sep 24, 2013 at 3:43 PM, Jose A. Lopes <[email protected]> wrote:
> 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... > Probably because we don't support disabling htools any more ;-). I thought we even want to get rid of the "disable-confd" part soon-ish, so it might make sense to clean those two up together. > > > > > > > > +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 > -- 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
