Just nitpicking:

On Thu, Jan 16, 2014 at 3:26 PM, Klaus Aehlig <[email protected]> wrote:

> In this way, scheduling decisions can depend on the configuration
> of the cluster. At the moment, this is only the maximal number
> jobs to be run in parallel, but in the future this will also include
> job filters.
>
> Signed-off-by: Klaus Aehlig <[email protected]>
> ---
>  src/Ganeti/JQScheduler.hs  | 8 +++++---
>  src/Ganeti/Query/Server.hs | 2 +-
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/src/Ganeti/JQScheduler.hs b/src/Ganeti/JQScheduler.hs
> index db80d80..f970075 100644
> --- a/src/Ganeti/JQScheduler.hs
> +++ b/src/Ganeti/JQScheduler.hs
> @@ -44,6 +44,7 @@ import Ganeti.BasicTypes
>  import Ganeti.Constants as C
>  import Ganeti.JQueue as JQ
>  import Ganeti.Logging
> +import Ganeti.Objects (ConfigData)
>  import Ganeti.Path
>  import Ganeti.Types
>  import Ganeti.Utils
> @@ -66,13 +67,14 @@ both, in particular when scheduling jobs to be handed
> over for execution.
>
>  data JQStatus = JQStatus
>    { jqJobs :: IORef Queue
> +  , jqConfig :: IORef (Result ConfigData)
>    }
>
>
> -emptyJQStatus :: IO JQStatus
> -emptyJQStatus = do
> +emptyJQStatus :: IORef (Result ConfigData) -> IO JQStatus
> +emptyJQStatus config = do
>    jqJ <- newIORef Queue {qEnqueued=[], qRunning=[]}
> -  return JQStatus { jqJobs=jqJ }
> +  return JQStatus { jqJobs=jqJ, jqConfig = config }
>

It'd be better to have a consistent assignment style across the whole
function, either 'x=y' or 'x = y', and also "{x..." vs "{ x...".

Otherwise LGTM, thanks (no need to resend)


>
>  -- | Apply a function on the running jobs.
>  onRunningJobs :: ([JobWithStat] -> [JobWithStat]) -> Queue -> Queue
> diff --git a/src/Ganeti/Query/Server.hs b/src/Ganeti/Query/Server.hs
> index e1ae45b..ae8fcc5 100644
> --- a/src/Ganeti/Query/Server.hs
> +++ b/src/Ganeti/Query/Server.hs
> @@ -396,7 +396,7 @@ prepMain _ _ = do
>    s <- describeError "binding to the Luxi socket"
>           Nothing (Just socket_path) $ getLuxiServer True socket_path
>    cref <- newIORef (Bad "Configuration not yet loaded")
> -  jq <- emptyJQStatus
> +  jq <- emptyJQStatus cref
>    return (s, cref, jq)
>
>  -- | Main function.
> --
> 1.8.5.2
>
>

Reply via email to