Great, so I'd like to propose this additional interdiff which adds a
separate module for ConfigState, a function for atomic modification of
ConfigState within WConfdMonad, and also adds a few comments:
diff --git a/Makefile.am b/Makefile.am
index 14052ca..0466569 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -774,6 +774,7 @@ HS_LIB_SRCS = \
src/Ganeti/UDSServer.hs \
src/Ganeti/Utils.hs \
src/Ganeti/VCluster.hs \
+ src/Ganeti/WConfd/ConfigState.hs \
src/Ganeti/WConfd/Core.hs \
src/Ganeti/WConfd/Monad.hs \
src/Ganeti/WConfd/Server.hs
diff --git a/src/Ganeti/WConfd/ConfigState.hs
b/src/Ganeti/WConfd/ConfigState.hs
new file mode 100644
index 0000000..db3d308
--- /dev/null
+++ b/src/Ganeti/WConfd/ConfigState.hs
@@ -0,0 +1,38 @@
+{-| Pure functions for manipulating the configuration state.
+
+-}
+
+{-
+
+Copyright (C) 2014 Google Inc.
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+02110-1301, USA.
+
+-}
+
+module Ganeti.WConfd.ConfigState
+ ( ConfigState
+ , mkConfigState
+ ) where
+
+-- | In future this data type will include the current configuration
+-- ('ConfigData') and the last 'FStat' of its file.
+data ConfigState = ConfigState
+
+-- | Creates a new configuration state.
+-- This method will expand as more fields are added to 'ConfigState'.
+mkConfigState :: ConfigState
+mkConfigState = ConfigState
diff --git a/src/Ganeti/WConfd/Monad.hs b/src/Ganeti/WConfd/Monad.hs
index 2f6a2c5..9df4016 100644
--- a/src/Ganeti/WConfd/Monad.hs
+++ b/src/Ganeti/WConfd/Monad.hs
@@ -35,13 +35,14 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor,
Boston, MA
-}
module Ganeti.WConfd.Monad
- ( ConfigState(..)
- , DaemonHandle(..)
+ ( DaemonHandle
+ , dhConfigPath
, mkDaemonHandle
, ClientState(..)
, WConfdMonadInt
, runWConfdMonadInt
, WConfdMonad
+ , modifyConfigState
) where
import Control.Applicative
@@ -56,23 +57,24 @@ import Ganeti.BasicTypes
import Ganeti.Errors
import Ganeti.Logging
import Ganeti.Types
+import Ganeti.WConfd.ConfigState
-- * Pure data types used in the monad
--- | In future this data type will include the current configuration
--- ('ConfigData') and the last 'FStat' of its file.
-data ConfigState = ConfigState
-
-- | The state of the daemon, capturing both the configuration state and
the
-- locking state.
--
-- Currently contains only the configuration state, the the locking state
will
-- go here in the future as well.
-data DaemonState = DaemonState ConfigState
+data DaemonState = DaemonState
+ { dsConfigState :: ConfigState
+ }
data DaemonHandle = DaemonHandle
{ dhDaemonState :: IORef DaemonState -- ^ The current state of the daemon
, dhConfigPath :: FilePath -- ^ The configuration file path
+ -- all static information that doesn't change during the life-time of the
+ -- daemon should go here;
-- all IDs of threads that do asynchronous work should probably also go
here
}
@@ -124,9 +126,22 @@ instance MonadBaseControl IO WConfdMonadInt where
instance MonadLog WConfdMonadInt where
logAt p = WConfdMonadInt . logAt p
+-- | Runs the internal part of the WConfdMonad monad on a given daemon
+-- handle.
runWConfdMonadInt :: WConfdMonadInt a -> DaemonHandle -> IO a
runWConfdMonadInt (WConfdMonadInt k) dhandle =
liftM fst $ evalRWST k dhandle Nothing
-- | The complete monad with error handling.
type WConfdMonad = ResultT GanetiException WConfdMonadInt
+
+-- * Basic functions in the monad
+
+-- | Atomically modifies the configuration state in the WConfdMonad.
+modifyConfigState :: (ConfigState -> (ConfigState, a)) -> WConfdMonad a
+modifyConfigState f = do
+ dh <- lift . WConfdMonadInt $ ask
+ -- TODO: Use lenses to modify the daemons state here
+ let mf ds = let (cs', r) = f (dsConfigState ds)
+ in (ds { dsConfigState = cs' }, r)
+ liftBase $ atomicModifyIORef (dhDaemonState dh) mf
On Fri, Feb 14, 2014 at 9:34 AM, Klaus Aehlig <[email protected]> wrote:
> > PS: Maybe the combination of 2. and 3. would be the best approach? Keep
> > Ganeti.WConfd.Monad slim and keep ConfigState in its dedicated module,
> > exporting all functions that are needed to read and modify it.
>
> Yes, that would be great.
>
> My only concern is, that it is hard to later change a data structure
> if all modules can access the internals. So exporting enough functions
> for manipulating is fine, but exporting a constructor often de-facto
> makes it unchangable, wich I think is dangerous for the ConfigState.
>
> Thanks,
> Klaus
>
> --
> Klaus Aehlig
> Google Germany GmbH, Dienerstr. 12, 80331 Muenchen
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
> Geschaeftsfuehrer: Graham Law, Christine Elizabeth Flores
>
