LGTM. Thanks, Jose
On Apr 07 19:00, Hrvoje Ribicic wrote: > After changes in later patches that check parameter combinations and set > defaults in the LU, the defaults should not be in the OpParams. > By setting them, we lose the ability to know when they have been supplied > and when they have not. > > Interdiff: > > diff --git a/src/Ganeti/OpParams.hs b/src/Ganeti/OpParams.hs > index 4e5e98e..40527c7 100644 > --- a/src/Ganeti/OpParams.hs > +++ b/src/Ganeti/OpParams.hs > @@ -1497,15 +1497,13 @@ pZeroingTimeoutFixed :: Field > pZeroingTimeoutFixed = > withDoc "The fixed part of time to wait before declaring the zeroing\ > \ operation to have failed" . > - defaultField [| C.helperVmStartup |] $ > - simpleField "zeroing_timeout_fixed" [t| Int |] > + optionalField $ simpleField "zeroing_timeout_fixed" [t| Int |] > > pZeroingTimeoutPerMiB :: Field > pZeroingTimeoutPerMiB = > withDoc "The variable part of time to wait before declaring the zeroing\ > \ operation to have failed, dependent on total size of disks" . > - defaultField [| C.zeroingTimeoutPerMib |] $ > - simpleField "zeroing_timeout_per_mib" [t| Double |] > + optionalField $ simpleField "zeroing_timeout_per_mib" [t| Double |] > > pTagsObject :: Field > pTagsObject = > > > > On Wed, Apr 2, 2014 at 4:53 PM, Jose A. Lopes <[email protected]> wrote: > > > LGTM. > > > > Thanks, > > Jose > > > > On Apr 02 13:30, Hrvoje Ribicic wrote: > > > This patch adds two parameters controlling the zeroing timeout - one > > > that is fixed and another that depends on the amount of data (size of > > > disks) to zero. > > > > > > Signed-off-by: Hrvoje Ribicic <[email protected]> > > > --- > > > src/Ganeti/Constants.hs | 14 ++++++++++++++ > > > src/Ganeti/OpCodes.hs | 2 ++ > > > src/Ganeti/OpParams.hs | 16 ++++++++++++++++ > > > test/hs/Test/Ganeti/OpCodes.hs | 3 ++- > > > 4 files changed, 34 insertions(+), 1 deletion(-) > > > > > > diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs > > > index 8be545a..8d2d3da 100644 > > > --- a/src/Ganeti/Constants.hs > > > +++ b/src/Ganeti/Constants.hs > > > @@ -4876,3 +4876,17 @@ debugModeConfidentialityWarning = > > > -- | The size of the file > > > statSize :: String > > > statSize = "size" > > > + > > > + > > > +-- * Helper VM-related timeouts > > > + > > > +-- | The default fixed timeout - needed to startup the helper VM > > > +helperVmStartup :: Int > > > +helperVmStartup = 5 * 60 > > > + > > > +-- | The zeroing timeout per MiB of disks to zero > > > +-- > > > +-- Determined by estimating that a disk writes at a relatively slow > > speed of 1/5 > > > +-- of the max speed of current drives > > > +zeroingTimeoutPerMib :: Double > > > +zeroingTimeoutPerMib = 1.0 / (100.0 / 5.0) > > > diff --git a/src/Ganeti/OpCodes.hs b/src/Ganeti/OpCodes.hs > > > index 873d18b..0cb840c 100644 > > > --- a/src/Ganeti/OpCodes.hs > > > +++ b/src/Ganeti/OpCodes.hs > > > @@ -778,6 +778,8 @@ $(genOpCode "OpCode" > > > , pX509KeyName > > > , pX509DestCA > > > , pZeroFreeSpace > > > + , pZeroingTimeoutFixed > > > + , pZeroingTimeoutPerMiB > > > ], > > > "instance_name") > > > , ("OpBackupRemove", > > > diff --git a/src/Ganeti/OpParams.hs b/src/Ganeti/OpParams.hs > > > index b222ef6..133a6f3 100644 > > > --- a/src/Ganeti/OpParams.hs > > > +++ b/src/Ganeti/OpParams.hs > > > @@ -220,6 +220,8 @@ module Ganeti.OpParams > > > , pX509KeyName > > > , pX509DestCA > > > , pZeroFreeSpace > > > + , pZeroingTimeoutFixed > > > + , pZeroingTimeoutPerMiB > > > , pTagSearchPattern > > > , pRestrictedCommand > > > , pReplaceDisksMode > > > @@ -1491,6 +1493,20 @@ pZeroFreeSpace = > > > withDoc "Whether to zero the free space on the disk of the instance" $ > > > defaultFalse "zero_free_space" > > > > > > +pZeroingTimeoutFixed :: Field > > > +pZeroingTimeoutFixed = > > > + withDoc "The fixed part of time to wait before declaring the zeroing\ > > > + \ operation to have failed" . > > > + defaultField [| C.helperVmStartup |] $ > > > + simpleField "zeroing_timeout_fixed" [t| Int |] > > > + > > > +pZeroingTimeoutPerMiB :: Field > > > +pZeroingTimeoutPerMiB = > > > + withDoc "The variable part of time to wait before declaring the > > zeroing\ > > > + \ operation to have failed, dependent on total size of > > disks" . > > > + defaultField [| C.zeroingTimeoutPerMib |] $ > > > + simpleField "zeroing_timeout_per_mib" [t| Double |] > > > + > > > pTagsObject :: Field > > > pTagsObject = > > > withDoc "Tag kind" $ > > > diff --git a/test/hs/Test/Ganeti/OpCodes.hs > > b/test/hs/Test/Ganeti/OpCodes.hs > > > index 6595546..da791ad 100644 > > > --- a/test/hs/Test/Ganeti/OpCodes.hs > > > +++ b/test/hs/Test/Ganeti/OpCodes.hs > > > @@ -401,7 +401,8 @@ instance Arbitrary OpCodes.OpCode where > > > OpCodes.OpBackupExport <$> genFQDN <*> return Nothing <*> > > > arbitrary <*> arbitrary <*> arbitrary <*> return Nothing <*> > > > arbitrary <*> arbitrary <*> arbitrary <*> arbitrary <*> > > > - genMaybe (pure []) <*> genMaybe genNameNE <*> arbitrary > > > + genMaybe (pure []) <*> genMaybe genNameNE <*> arbitrary <*> > > > + arbitrary <*> arbitrary > > > "OP_BACKUP_REMOVE" -> > > > OpCodes.OpBackupRemove <$> genFQDN <*> return Nothing > > > "OP_TEST_ALLOCATOR" -> > > > -- > > > 1.9.1.423.g4596e3a > > > > > > > -- > > 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 > > -- 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
