> +fnToVal :: String -> IO JSValue
> +fnToVal fn
> +  | null fn = return $ okWithDetails
> +      "No file specified for diagnose data collector"
> +  | not $ isValid fn = return $ okWithDetails
> +      "Invalid filename specified for diagnose data collector"
> +  | takeFileName fn /= fn = return $ okWithDetails
> +      "Filepaths cannot be specified for diagnose data collector"
> +  | otherwise = do
> +      let fp = dataCollectorDiagnoseDirectory </> fn
> +      exists <- doesFileExist fp
> +      if exists
> +        then do
> +          fs <- getFileStatus fp
> +          let maxFileMode = foldl1 unionFileModes [ ownerModes
> +                                                  , groupReadMode
> +                                                  , groupExecuteMode
> +                                                  , otherReadMode
> +                                                  , otherExecuteMode
> +                                                  , regularFileMode
> +                                                  ]
> +              isSubSetOf m1 m2 = m1 `intersectFileModes` m2 == m1
> +          case () of _
> +                       | fileOwner fs /= 0 -> return $ okWithDetails
> +                       "File for diagnose data collector must be owned by 
> root"
> +                       | fileGroup fs /= 0 -> return $ okWithDetails
> +                       "File for diagnose data collector must have group 
> root"
> +                       | not $ isRegularFile fs ->
> +                       return . okWithDetails $
> +                       "File for diagnose data collector " ++
> +                       "must be a regular file"
> +                       | not $ isSubSetOf (fileMode fs) maxFileMode ->
> +                       return . okWithDetails $
> +                       "File for diagnose data collector " ++
> +                       "must have permissions 755 or stricter"
> +                       | not $ isSubSetOf ownerExecuteMode (fileMode fs) ->
> +                       return . okWithDetails $
> +                       "File for diagnose data collector " ++
> +                       "must be executable by owner"
> +                       | otherwise -> do

Please properly format that case statement; at the very least the coding style 
should
be consistent within the a single newly-added function!

> +                           r <- fmap decode (readProcess fp [] "")
> +                           case r of
> +                             Ok val -> return val
> +                             Error str -> return . okWithDetails $
> +                               "Could not parse result: " ++ str
> +        else return $ okWithDetails
> +          "File specified for diagnose data collector does not exist"
> +
> +buildJsonReport :: IO JSValue
> +buildJsonReport = fmap (genericResult okWithDetails id) . runResultT $ do
> +  configData <- ResultT (clusterConfFile >>= loadConfig)
> +  lift . fnToVal . clusterDiagnoseDataCollectorFilename $
> +    configCluster configData
> +
> +-- | The data exported by the data collector, taken from the default 
> location.
> +dcReport :: IO DCReport
> +dcReport = buildJsonReport >>=
> +  buildReport dcName dcVersion dcFormatVersion dcCategory dcKind
> diff --git a/src/Ganeti/DataCollectors/Types.hs 
> b/src/Ganeti/DataCollectors/Types.hs
> index 8b60be1..3bd31b3 100644
> --- a/src/Ganeti/DataCollectors/Types.hs
> +++ b/src/Ganeti/DataCollectors/Types.hs
> @@ -68,7 +68,7 @@ import Ganeti.THH
>  import Ganeti.Utils (getCurrentTimeUSec)
>  
>  -- | The possible classes a data collector can belong to.
> -data DCCategory = DCInstance | DCStorage | DCDaemon | DCHypervisor
> +data DCCategory = DCInstance | DCStorage | DCDaemon | DCHypervisor | DCNode
>    deriving (Show, Eq, Read, Enum, Bounded)

Adding a new category also requires extending the catFromName function in 
Monitoring/Server;
otherwise it won't be possible to get the individual report. Did you follow my 
advise to
actually test your code before sending it out for review??

-- 
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

Reply via email to