On Mon, Feb 04, 2013 at 10:44:42AM +0100, Michele Tartara wrote:
> In order to fetch information about the uptime of the VMs running in Xen,
> we need to analyze the output of the "xm uptime" command.
> 
> This commit adds the parser to do that, and its tests.
> 
> Signed-off-by: Michele Tartara <[email protected]>
> ---
>  Makefile.am                                        |  3 +
>  src/Ganeti/Hypervisor/Xen/Types.hs                 | 13 ++++
>  src/Ganeti/Hypervisor/Xen/XmUptimeParser.hs        | 52 +++++++++++++
>  test/data/xen-xm-uptime-3.0.txt                    |  3 +
>  .../Test/Ganeti/Hypervisor/Xen/XmUptimeParser.hs   | 88 
> ++++++++++++++++++++++
>  test/hs/htest.hs                                   |  2 +
>  6 files changed, 161 insertions(+)
>  create mode 100644 src/Ganeti/Hypervisor/Xen/XmUptimeParser.hs
>  create mode 100644 test/data/xen-xm-uptime-3.0.txt
>  create mode 100644 test/hs/Test/Ganeti/Hypervisor/Xen/XmUptimeParser.hs
> 
> diff --git a/Makefile.am b/Makefile.am
> index 9af1f87..f98802a 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -521,6 +521,7 @@ HS_LIB_SRCS = \
>       src/Ganeti/HTools/Program/Main.hs \
>       src/Ganeti/HTools/Types.hs \
>       src/Ganeti/Hypervisor/Xen/XmListParser.hs \
> +     src/Ganeti/Hypervisor/Xen/XmUptimeParser.hs \
>       src/Ganeti/Hypervisor/Xen/Types.hs \
>       src/Ganeti/Hash.hs \
>       src/Ganeti/JQueue.hs \
> @@ -572,6 +573,7 @@ HS_TEST_SRCS = \
>       test/hs/Test/Ganeti/HTools/PeerMap.hs \
>       test/hs/Test/Ganeti/HTools/Types.hs \
>       test/hs/Test/Ganeti/Hypervisor/Xen/XmListParser.hs \
> +     test/hs/Test/Ganeti/Hypervisor/Xen/XmUptimeParser.hs \
>       test/hs/Test/Ganeti/JSON.hs \
>       test/hs/Test/Ganeti/Jobs.hs \
>       test/hs/Test/Ganeti/JQueue.hs \
> @@ -1053,6 +1055,7 @@ TEST_FILES = \
>       test/data/xen-xm-list-4.0.1-dom0-only.txt \
>       test/data/xen-xm-list-4.0.1-four-instances.txt \
>       test/data/xen-xm-list-long-3.0.txt \
> +     test/data/xen-xm-uptime-3.0.txt \
>       test/py/ganeti-cli.test \
>       test/py/gnt-cli.test \
>       test/py/import-export_unittest-helper
> diff --git a/src/Ganeti/Hypervisor/Xen/Types.hs 
> b/src/Ganeti/Hypervisor/Xen/Types.hs
> index 3b10b30..b2e5745 100644
> --- a/src/Ganeti/Hypervisor/Xen/Types.hs
> +++ b/src/Ganeti/Hypervisor/Xen/Types.hs
> @@ -27,8 +27,11 @@ module Ganeti.Hypervisor.Xen.Types
>    , Domain(..)
>    , FromConfig(..)
>    , isAlmostEqual
> +  , UptimeInfo(..)
>    ) where
>  
> +import Text.Printf
> +
>  import Ganeti.BasicTypes
>  import Ganeti.Objects
>  
> @@ -104,3 +107,13 @@ instance FromConfig [Config] where
>    fromConfig c =
>      Bad $ "Unable to extract a List from this configuration: "
>        ++ show c
> +
> +-- Data type representing the information that can be obtained from "xm 
> uptime"
> +data UptimeInfo = UptimeInfo
> +  { uInfoName :: String
> +  , uInfoID :: Int
> +  , uInfoUptime :: String
> +  } deriving (Eq)
> +
> +instance Show UptimeInfo where
> +  show (UptimeInfo name idNum uptime) = printf "%s\t%d\t%s" name idNum uptime

Again, does this need to be Show or do you simply want a pretty-print
format?

> diff --git a/src/Ganeti/Hypervisor/Xen/XmUptimeParser.hs 
> b/src/Ganeti/Hypervisor/Xen/XmUptimeParser.hs
> new file mode 100644
> index 0000000..bf2e8f7
> --- /dev/null
> +++ b/src/Ganeti/Hypervisor/Xen/XmUptimeParser.hs
> @@ -0,0 +1,52 @@

I'm a bit… not convinced here that one-xm-command-per-file model is
good. This file, for example, has only 52 lines, which IMHO is too short
to warrant a new module.

Wouldn't it make more sense to keep all xm-related functionality
(remember that xl will change things yet again, I think) into a single
Xm.hs file?

> diff --git a/test/hs/Test/Ganeti/Hypervisor/Xen/XmUptimeParser.hs 
> b/test/hs/Test/Ganeti/Hypervisor/Xen/XmUptimeParser.hs
> new file mode 100644
> index 0000000..71e5aad
> --- /dev/null
> +++ b/test/hs/Test/Ganeti/Hypervisor/Xen/XmUptimeParser.hs
> @@ -0,0 +1,88 @@
> +{-# LANGUAGE TemplateHaskell #-}
> +{-# OPTIONS_GHC -fno-warn-orphans #-}
> +
> +{-| Unittests for "xm list --long" parser -}

:)

> +
> +{-
> +
> +Copyright (C) 2013 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 Test.Ganeti.Hypervisor.Xen.XmUptimeParser 
> (testHypervisor_Xen_XmUptimeParser) where
> +
> +import Test.HUnit
> +import Test.QuickCheck as QuickCheck hiding (Result)
> +
> +import Test.Ganeti.TestHelper
> +import Test.Ganeti.TestCommon
> +
> +import qualified Data.Attoparsec.Text as A
> +import qualified Data.Map as Map
> +import Data.Text (pack)
> +import Text.Printf
> +
> +import Ganeti.Hypervisor.Xen.Types
> +import Ganeti.Hypervisor.Xen.XmUptimeParser
> +
> +{-# ANN module "HLint: ignore Use camelCase" #-}
> +
> +-- | Generates an arbitrary "xm uptime" output
> +instance Arbitrary UptimeInfo where
> +  arbitrary = do
> +    name <- genFQDN
> +    idNum <- (arbitrary :: Gen Int) `suchThat` (>= 0)
> +    days <- (arbitrary :: Gen Int) `suchThat` (>= 0)

You can/should use the NonNegative instances here:

       NonNegative idNum <- arbitrary
       NonNegative days <- arbitrary

> +    hours <- choose (0, 23) :: Gen Int
> +    mins <- choose (0, 59) :: Gen Int
> +    secs <- choose (0, 59) :: Gen Int
> +    let uptime :: String
> +        uptime =
> +          if days == 0
> +            then printf "%d days, %d:%d:%d" days hours mins secs
> +            else printf "%d:%d:%d" hours mins secs

Wrong/reverted if test? if days ≠ 0 … ?

> +    return $ UptimeInfo name idNum uptime

thanks,
iustin

Reply via email to