On Fri, Sep 27, 2013 at 11:17 AM, Klaus Aehlig <[email protected]> wrote:
> On Thu, Sep 26, 2013 at 04:52:27PM +0000, Michele Tartara wrote: > > The data collectors should be able to provide as much information as > possible > > even when the system is badly degraded. This patch modifies the instance > status > > collector for xen so that it can keep providing as much data as > possible, even > > when some of the queries it performs fail, by removing exitIfBad calls > and > > substituting them with logging and returning empty fields instead. > > > > Signed-off-by: Michele Tartara <[email protected]> > > --- > > src/Ganeti/DataCollectors/InstStatus.hs | 20 ++++++++++++++------ > > src/Ganeti/Hypervisor/Xen.hs | 30 > +++++++++++++++++++++--------- > > 2 files changed, 35 insertions(+), 15 deletions(-) > > > > > > + d <- getInferredDomInfo > > + reportData <- > > + case d of > > + BT.Ok domains -> do > > + uptimes <- getUptimeInfo > > + let primaryInst = fst inst > > + iStatus <- mapM (buildStatus domains uptimes) primaryInst > > + let globalStatus = computeGlobalStatus iStatus > > + return $ ReportData iStatus globalStatus > > + BT.Bad m -> > > + (return . ReportData []) . DCStatus DCSCBad $ > > redundant parentheses? > Yes, and hlint didn't see them. I'll remove them before pushing. Interdiff: diff --git a/src/Ganeti/DataCollectors/InstStatus.hs b/src/Ganeti/DataCollectors/InstStatus.hs index 0430274..ed898d9 100644 --- a/src/Ganeti/DataCollectors/InstStatus.hs +++ b/src/Ganeti/DataCollectors/InstStatus.hs @@ -192,7 +192,7 @@ buildInstStatusReport srvAddr srvPort = do let globalStatus = computeGlobalStatus iStatus return $ ReportData iStatus globalStatus BT.Bad m -> - (return . ReportData []) . DCStatus DCSCBad $ + return . ReportData [] . DCStatus DCSCBad $ "Unable to receive the list of instances: " ++ m let jsonReport = J.showJSON reportData buildReport dcName dcVersion dcFormatVersion dcCategory dcKind jsonReport > > + "Unable to receive the list of instances: " ++ m > > + let jsonReport = J.showJSON reportData > > buildReport dcName dcVersion dcFormatVersion dcCategory dcKind > jsonReport > > > > -- | Main function. > > diff --git a/src/Ganeti/Hypervisor/Xen.hs b/src/Ganeti/Hypervisor/Xen.hs > > index 06c928e..3fbffa7 100644 > > --- a/src/Ganeti/Hypervisor/Xen.hs > > +++ b/src/Ganeti/Hypervisor/Xen.hs > > @@ -40,21 +40,25 @@ import qualified Ganeti.BasicTypes as BT > > import qualified Ganeti.Constants as C > > import Ganeti.Hypervisor.Xen.Types > > import Ganeti.Hypervisor.Xen.XmParser > > +import Ganeti.Logging > > import Ganeti.Utils > > > > > > -- | Get information about the current Xen domains as a map where the > domain > > -- name is the key. This only includes the information made available > by Xen > > -- itself. > > -getDomainsInfo :: IO (Map.Map String Domain) > > +getDomainsInfo :: IO (BT.Result (Map.Map String Domain)) > > getDomainsInfo = do > > contents <- > > - ((E.try $ readProcess C.xenCmdXm ["list", "--long"] "") > > - :: IO (Either IOError String)) >>= > > - exitIfBad "running command" . either (BT.Bad . show) BT.Ok > > - case A.parseOnly xmListParser $ pack contents of > > - Left msg -> exitErr msg > > - Right dom -> return dom > > + (E.try $ readProcess C.xenCmdXm ["list", "--long"] "") > > + :: IO (Either IOError String) > > + return $ > > + either (BT.Bad . show) ( > > + \c -> > > + case A.parseOnly xmListParser $ pack c of > > + Left msg -> BT.Bad msg > > + Right dom -> BT.Ok dom > > + ) contents > > The > > case ... of > Left msg -> BT.Bad msg > Right dom -> BT.Ok dom > > could also be written point free as > > either BT.Bad BT.Ok $ ... > > But I'm not sure whether this improves the readability in this case. > > Either way, LGTM. Being already inside another "either", I think it would become less readable. I'll leave it as it is. Thanks for the suggestion, though. Cheers, Michele -- 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
