On Thu, Apr 17, 2014 at 12:49PM, Jose A. Lopes wrote:
> On Apr 16 15:19, Ilias Tsitsimpis wrote:
> > Now that disks are top level citizens in config,
> > they need a timestamp and a serial_no slot.
> > 
> > Signed-off-by: Ilias Tsitsimpis <[email protected]>
> > ---
> >  lib/objects.py                  |  9 ++++++++-
> >  src/Ganeti/Objects.hs           |  7 ++++++-
> >  test/data/instance-prim-sec.txt | 15 ++++++++++++---
> >  test/hs/Test/Ganeti/Objects.hs  | 21 ++++++++++++++-------
> >  test/py/ganeti.rpc_unittest.py  |  6 ++++++
> >  5 files changed, 46 insertions(+), 12 deletions(-)
> > 
> > diff --git a/lib/objects.py b/lib/objects.py
> > index bd72e07..1b3c7d2 100644
> > --- a/lib/objects.py
> > +++ b/lib/objects.py
> > @@ -532,7 +532,8 @@ class NIC(ConfigObject):
> >  class Disk(ConfigObject):
> >    """Config object representing a block device."""
> >    __slots__ = (["name", "dev_type", "logical_id", "children", "iv_name",
> > -                "size", "mode", "params", "spindles", "pci"] + _UUID +
> > +                "size", "mode", "params", "spindles", "pci", "serial_no"] +
> > +               _UUID + _TIMESTAMPS +
> 
> Format this vertically so it is consistent with the other classes.

Interdiff:

diff --git a/lib/objects.py b/lib/objects.py
index 3c71483..57fa988 100644
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -530,12 +530,22 @@ class NIC(ConfigObject):
 
 class Disk(ConfigObject):
   """Config object representing a block device."""
-  __slots__ = (["name", "dev_type", "logical_id", "children", "iv_name",
-                "size", "mode", "params", "spindles", "pci", "serial_no"] +
-               _UUID + _TIMESTAMPS +
-               # dynamic_params is special. It depends on the node this 
instance
-               # is sent to, and should not be persisted.
-               ["dynamic_params"])
+  __slots__ = [
+    "name",
+    "dev_type",
+    "logical_id",
+    "children",
+    "iv_name",
+    "size",
+    "mode",
+    "params",
+    "spindles",
+    "pci",
+    "serial_no",
+    # dynamic_params is special. It depends on the node this instance
+    # is sent to, and should not be persisted.
+    "dynamic_params"
+    ] + _UUID + _TIMESTAMPS
 
   def _ComputeAllNodes(self):
     """Compute the list of all nodes covered by a device and its children."""


Thanks,
Ilias


> Rest LGTM.
> 
> Thanks,
> Jose
> 
> >                 # dynamic_params is special. It depends on the node this 
> > instance
> >                 # is sent to, and should not be persisted.
> >                 ["dynamic_params"])
> > @@ -877,6 +878,12 @@ class Disk(ConfigObject):
> >        self.params = {}
> >  
> >      # add here config upgrade for this disk
> > +    if self.serial_no is None:
> > +      self.serial_no = 1
> > +    if self.mtime is None:
> > +      self.mtime = time.time()
> > +    if self.ctime is None:
> > +      self.ctime = time.time()
> >  
> >      # map of legacy device types (mapping differing LD constants to new
> >      # DT constants)
> > diff --git a/src/Ganeti/Objects.hs b/src/Ganeti/Objects.hs
> > index 3cf1ce8..8a0c5ac 100644
> > --- a/src/Ganeti/Objects.hs
> > +++ b/src/Ganeti/Objects.hs
> > @@ -405,6 +405,9 @@ data Disk = Disk
> >    , diskName       :: Maybe String
> >    , diskSpindles   :: Maybe Int
> >    , diskUuid       :: String
> > +  , diskSerial     :: Int
> > +  , diskCtime      :: ClockTime
> > +  , diskMtime      :: ClockTime
> >    } deriving (Show, Eq)
> >  
> >  $(buildObjectSerialisation "Disk" $
> > @@ -417,7 +420,9 @@ $(buildObjectSerialisation "Disk" $
> >    , optionalField $ simpleField "name" [t| String |]
> >    , optionalField $ simpleField "spindles" [t| Int |]
> >    ]
> > -  ++ uuidFields)
> > +  ++ uuidFields
> > +  ++ serialFields
> > +  ++ timeStampFields)
> >  
> >  instance UuidObject Disk where
> >    uuidOf = diskUuid
> > diff --git a/test/data/instance-prim-sec.txt 
> > b/test/data/instance-prim-sec.txt
> > index b159439..b27fd5c 100644
> > --- a/test/data/instance-prim-sec.txt
> > +++ b/test/data/instance-prim-sec.txt
> > @@ -17,7 +17,10 @@
> >               "df9ff3f6-a833-48ff-8bd5-bff2eaeab759.disk0_data"
> >             ],
> >             "size": 1024,
> > -           "uuid": "eaff6322-1bfb-4d59-b306-4535730917cc"
> > +           "uuid": "eaff6322-1bfb-4d59-b306-4535730917cc",
> > +           "serial_no": 1,
> > +           "ctime": 1372838946.2599809,
> > +           "mtime": 1372838946.2599809
> >           },
> >           {
> >             "dev_type": "plain",
> > @@ -31,7 +34,10 @@
> >               "df9ff3f6-a833-48ff-8bd5-bff2eaeab759.disk0_meta"
> >             ],
> >             "size": 128,
> > -           "uuid": "bf512e95-2a49-4cb3-8d1f-30a503f6bf1b"
> > +           "uuid": "bf512e95-2a49-4cb3-8d1f-30a503f6bf1b",
> > +           "serial_no": 1,
> > +           "ctime": 1372838946.2599809,
> > +           "mtime": 1372838946.2599809
> >           }
> >         ],
> >         "dev_type": "drbd",
> > @@ -55,7 +61,10 @@
> >           "9bdb15fb7ab6bb4610a313d654ed4d0d2433713e"
> >         ],
> >         "size": 1024,
> > -       "uuid": "5d61e205-bf89-4ba8-a319-589b7bb7419e"
> > +       "uuid": "5d61e205-bf89-4ba8-a319-589b7bb7419e",
> > +       "serial_no": 1,
> > +       "ctime": 1372838946.2599809,
> > +       "mtime": 1372838946.2599809
> >       }
> >     ],
> >     "disks_active": true,
> > diff --git a/test/hs/Test/Ganeti/Objects.hs b/test/hs/Test/Ganeti/Objects.hs
> > index 1a31b55..8653fc5 100644
> > --- a/test/hs/Test/Ganeti/Objects.hs
> > +++ b/test/hs/Test/Ganeti/Objects.hs
> > @@ -51,6 +51,7 @@ import qualified Data.Map as Map
> >  import Data.Maybe (fromMaybe)
> >  import qualified Data.Set as Set
> >  import GHC.Exts (IsString(..))
> > +import System.Time (ClockTime(..))
> >  import qualified Text.JSON as J
> >  
> >  import Test.Ganeti.TestHelper
> > @@ -93,6 +94,7 @@ instance Arbitrary DiskLogicalId where
> >  instance Arbitrary Disk where
> >    arbitrary = Disk <$> arbitrary <*> pure [] <*> arbitrary
> >                     <*> arbitrary <*> arbitrary <*> arbitrary
> > +                   <*> arbitrary <*> arbitrary <*> arbitrary
> >                     <*> arbitrary <*> arbitrary
> >  
> >  -- FIXME: we should generate proper values, >=0, etc., but this is
> > @@ -185,8 +187,10 @@ genDiskWithChildren num_children = do
> >    name <- genMaybe genName
> >    spindles <- arbitrary
> >    uuid <- genName
> > -  let disk = Disk logicalid children ivname size mode name spindles uuid
> > -  return disk
> > +  serial <- arbitrary
> > +  time <- arbitrary
> > +  return $
> > +    Disk logicalid children ivname size mode name spindles uuid serial 
> > time time
> >  
> >  genDisk :: Gen Disk
> >  genDisk = genDiskWithChildren 3
> > @@ -555,9 +559,10 @@ caseIncludeLogicalIdPlain :: HUnit.Assertion
> >  caseIncludeLogicalIdPlain =
> >    let vg_name = "xenvg" :: String
> >        lv_name = "1234sdf-qwef-2134-asff-asd2-23145d.data" :: String
> > +      time = TOD 0 0
> >        d =
> >          Disk (LIDPlain vg_name lv_name) [] "diskname" 1000 DiskRdWr
> > -          Nothing Nothing "asdfgr-1234-5123-daf3-sdfw-134f43"
> > +          Nothing Nothing "asdfgr-1234-5123-daf3-sdfw-134f43" 0 time time
> >    in
> >      HUnit.assertBool "Unable to detect that plain Disk includes logical 
> > ID" $
> >        includesLogicalId vg_name lv_name d
> > @@ -567,15 +572,16 @@ caseIncludeLogicalIdDrbd :: HUnit.Assertion
> >  caseIncludeLogicalIdDrbd =
> >    let vg_name = "xenvg" :: String
> >        lv_name = "1234sdf-qwef-2134-asff-asd2-23145d.data" :: String
> > +      time = TOD 0 0
> >        d =
> >          Disk
> >            (LIDDrbd8 "node1.example.com" "node2.example.com" 2000 1 5 
> > "secret")
> >            [ Disk (LIDPlain "onevg" "onelv") [] "disk1" 1000 DiskRdWr 
> > Nothing
> > -              Nothing "145145-asdf-sdf2-2134-asfd-534g2x"
> > +              Nothing "145145-asdf-sdf2-2134-asfd-534g2x" 0 time time
> >            , Disk (LIDPlain vg_name lv_name) [] "disk2" 1000 DiskRdWr 
> > Nothing
> > -              Nothing "6gd3sd-423f-ag2j-563b-dg34-gj3fse"
> > +              Nothing "6gd3sd-423f-ag2j-563b-dg34-gj3fse" 0 time time
> >            ] "diskname" 1000 DiskRdWr Nothing Nothing
> > -          "asdfgr-1234-5123-daf3-sdfw-134f43"
> > +          "asdfgr-1234-5123-daf3-sdfw-134f43" 0 time time
> >    in
> >      HUnit.assertBool "Unable to detect that plain Disk includes logical 
> > ID" $
> >        includesLogicalId vg_name lv_name d
> > @@ -585,9 +591,10 @@ caseNotIncludeLogicalIdPlain :: HUnit.Assertion
> >  caseNotIncludeLogicalIdPlain =
> >    let vg_name = "xenvg" :: String
> >        lv_name = "1234sdf-qwef-2134-asff-asd2-23145d.data" :: String
> > +      time = TOD 0 0
> >        d =
> >          Disk (LIDPlain "othervg" "otherlv") [] "diskname" 1000 DiskRdWr
> > -          Nothing Nothing "asdfgr-1234-5123-daf3-sdfw-134f43"
> > +          Nothing Nothing "asdfgr-1234-5123-daf3-sdfw-134f43" 0 time time
> >    in
> >      HUnit.assertBool "Unable to detect that plain Disk includes logical 
> > ID" $
> >        not (includesLogicalId vg_name lv_name d)
> > diff --git a/test/py/ganeti.rpc_unittest.py b/test/py/ganeti.rpc_unittest.py
> > index 89b00a1..26f1069 100755
> > --- a/test/py/ganeti.rpc_unittest.py
> > +++ b/test/py/ganeti.rpc_unittest.py
> > @@ -884,18 +884,24 @@ class TestRpcRunner(unittest.TestCase):
> >      self.assertEqual(result["hvparams"][constants.HT_KVM], {
> >        constants.HV_BOOT_ORDER: "xyz",
> >        })
> > +    del result["disks"][0]["ctime"]
> > +    del result["disks"][0]["mtime"]
> > +    del result["disks"][1]["ctime"]
> > +    del result["disks"][1]["mtime"]
> >      self.assertEqual(result["disks"], [{
> >        "dev_type": constants.DT_PLAIN,
> >        "dynamic_params": {},
> >        "size": 4096,
> >        "logical_id": ("vg", "disk6120"),
> >        "params": constants.DISK_DT_DEFAULTS[inst.disk_template],
> > +      "serial_no": 1,
> >        }, {
> >        "dev_type": constants.DT_PLAIN,
> >        "dynamic_params": {},
> >        "size": 1024,
> >        "logical_id": ("vg", "disk8508"),
> >        "params": constants.DISK_DT_DEFAULTS[inst.disk_template],
> > +      "serial_no": 1,
> >        }])
> >  
> >      self.assertTrue(compat.all(disk.params == {} for disk in inst.disks),
> > -- 
> > 1.9.1
> > 
> 
> -- 
> Jose Antonio Lopes
> Ganeti Engineering
> 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
> Steuernummer: 48/725/00206
> Umsatzsteueridentifikationsnummer: DE813741370

-- 
Ilias Tsitsimpis
OpenPGP public key ID:
pub  4096R/25F3E3EE 2012-11-02 Ilias Tsitsimpis <[email protected]>
  Key fingerprint = 1986 21F5 7024 9B25 F4E2  4EF7 6FB8 90BA 25F3 E3EE


Tiger got to hunt, Bird got to fly;
Lisper got to sit and wonder, (Y (Y Y))?

Tiger got to sleep, Bird got to land;
Lisper got to tell himself he understand.

    — Kurt Vonnegut, modified by Darius Bacon

Attachment: signature.asc
Description: Digital signature

Reply via email to