Re: [PATCH master 07/52] More reshuffling of code

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 Following the split Types/BasicTypes, we can remove the last
 JSON-related stuff from Utils.hs, and do some more cleanup.
 ---
  htools/Ganeti/BasicTypes.hs   |    6 ++
  htools/Ganeti/HTools/JSON.hs  |   12 
  htools/Ganeti/HTools/Luxi.hs  |    2 +-
  htools/Ganeti/HTools/QC.hs    |    2 +-
  htools/Ganeti/HTools/Types.hs |    6 ++
  htools/Ganeti/HTools/Utils.hs |   22 --
  6 files changed, 26 insertions(+), 24 deletions(-)

LGTM

René


Re: [PATCH master 08/52] Stop exporting JSON functionality from Utils.hs

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 This completes the Utils/JSON split started in commit f047f90f. The
 import graph should be cleaner now.
 ---
  htools/Ganeti/HTools/IAlloc.hs |    2 +-
  htools/Ganeti/HTools/Luxi.hs   |    3 +--
  htools/Ganeti/HTools/QC.hs     |    5 +++--
  htools/Ganeti/HTools/Rapi.hs   |    2 +-
  htools/Ganeti/HTools/Utils.hs  |   14 --
  htools/Ganeti/Luxi.hs          |    2 +-
  htools/Ganeti/OpCodes.hs       |    3 +--
  7 files changed, 8 insertions(+), 23 deletions(-)

LGTM.

René


Re: [PATCH master 09/52] Add object definitions for the ispec and ipolicy

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 ---
  htools/Ganeti/HTools/Types.hs |   22 +-
  htools/Ganeti/THH.hs          |   10 +-
  2 files changed, 26 insertions(+), 6 deletions(-)

I'm never used template Haskell or anything related to it, but LGTM
from what I can read.

René


Re: [PATCH master 10/52] Improve convert-constants to handle dictionaries

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 The two main drawbacks for convert-constants are the fact that it
 can't handle sets/frozensets (mainly due to the fact that I don't know
 how useful this would be to the Haskell code) and that it cannot
 export dictionaries.

 To fix the second case, the current patch changes the code to support
 flattening (potentially nested) dictionaries into single name
 space. Yes, this could generate conflicts, but they will be detected
 at compile time.
 ---
  autotools/convert-constants |   70 +-
  1 files changed, 48 insertions(+), 22 deletions(-)

LGTM.

René


Re: [PATCH master 11/52] More improvements to convert-constants

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 This prepares for tuple and other conversions.
 ---
  autotools/convert-constants |   41 ++---
  1 files changed, 26 insertions(+), 15 deletions(-)

LGTM.

René


Re: [PATCH master 12/52] Add support for tuples in convert-constants

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 ---
  autotools/convert-constants |   11 +++
  1 files changed, 11 insertions(+), 0 deletions(-)

LGTM.

René


Re: [PATCH master 13/52] Add support for lists/frozensets in convert-constants

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 Unfortunately, we only support lists of simple types, and not even
 lists of tuples. If we actually needed those, it would be possible to
 implement them, with a bit more complexity in the converter.
 ---
  autotools/convert-constants |   20 
  1 files changed, 20 insertions(+), 0 deletions(-)

LGTM :)

René


Re: [PATCH master 14/52] Add support for RE patterns to convert constants

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 This is a trivial conversion.
 ---
  autotools/convert-constants |   10 ++
  1 files changed, 10 insertions(+), 0 deletions(-)

 diff --git a/autotools/convert-constants b/autotools/convert-constants
 index 89f9f00..ae36774 100755
 --- a/autotools/convert-constants
 +++ b/autotools/convert-constants
 @@ -27,8 +27,12 @@ import re
  from ganeti import constants
  from ganeti import compat

 +#: Constant name regex
  CONSTANT_RE = re.compile(^[A-Z][A-Z0-9_-]+$)

 +#: The type of regex objects
 +RE_TYPE = type(CONSTANT_RE)
 +

  def NameRules(name):
   Converts the upper-cased Python name to Haskell camelCase.
 @@ -129,6 +133,12 @@ def ConvertVariable(name, value):
         lines.append(-- | Skipped list/set %s, is not homogeneous % name)
     else:
       lines.append(-- | Skipped list/set %s, cannot convert all elems % 
 name)
 +  elif isinstance(value, RE_TYPE):
 +    tvs = HaskellTypeVal(value.pattern)

According to the doc, this is always a string. Any reason to anyway
going over HaskellTypeVal?

Rest LGTM

René


Re: [PATCH master 15/52] Add default ipolicy declarations

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 ---
  htools/Ganeti/HTools/Types.hs |   35 +++
  1 files changed, 35 insertions(+), 0 deletions(-)

LGTM.

René


Re: [PATCH master 16/52] Extend ClusterData with the cluster instance policy

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 This attribute is always initialised to the default, and is not (yet)
 read/saved in the various backends.
 ---
  htools/Ganeti/HTools/IAlloc.hs         |    4 ++--
  htools/Ganeti/HTools/Loader.hs         |    4 +++-
  htools/Ganeti/HTools/Luxi.hs           |    2 +-
  htools/Ganeti/HTools/Program/Hbal.hs   |    4 ++--
  htools/Ganeti/HTools/Program/Hscan.hs  |    4 ++--
  htools/Ganeti/HTools/Program/Hspace.hs |    4 ++--
  htools/Ganeti/HTools/QC.hs             |    2 +-
  htools/Ganeti/HTools/Rapi.hs           |    2 +-
  htools/Ganeti/HTools/Simu.hs           |    2 +-
  htools/Ganeti/HTools/Text.hs           |    4 ++--
  10 files changed, 17 insertions(+), 15 deletions(-)

LGTM.

René


Re: [PATCH master 17/52] Load cluster ipolicy via Luxi

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 Also show it in hbal's verbose output (helpful for debugging).
 ---
  htools/Ganeti/HTools/Luxi.hs         |   16 ++--
  htools/Ganeti/HTools/Program/Hbal.hs |    5 +++--
  2 files changed, 13 insertions(+), 8 deletions(-)

LGTM.

René


Re: [PATCH master 3/4] set-mem rapi and tests

2012-01-11 Thread Iustin Pop
On Tue, Jan 10, 2012 at 06:28:51PM +, Guido Trotter wrote:
 We still need --running-mem: this is meant for ballooning without changing
 min and max, which are the only values we currently save.

Guid, can you confirm this is correct?

- we have min/max mem, which are config values
- we have oper_ram, which is the current running memory (between
  min/max)
- currently gnt-instance modify only allows changing min/max _in the
  config_
- we want to be able to modify min/max at runtime and also oper_ram at
  runtime

iustin

 On Jan 10, 2012 3:47 PM, Michael Hanselmann han...@google.com wrote:
 
  Am 10. Januar 2012 12:29 schrieb Iustin Pop ius...@google.com:
   Hmm, right. But then I'd make it a different key (e.g. running-mem or
   such), but still in instance modify.
 
  Or add a new parameter “--live” to apply the changes right away.
 
  Michael
 


Re: [PATCH master 09/52] Add object definitions for the ispec and ipolicy

2012-01-11 Thread Iustin Pop
On Wed, Jan 11, 2012 at 10:17:38AM +0100, René Nussbaumer wrote:
 On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
  ---
   htools/Ganeti/HTools/Types.hs |   22 +-
   htools/Ganeti/THH.hs          |   10 +-
   2 files changed, 26 insertions(+), 6 deletions(-)
 
 I'm never used template Haskell or anything related to it, but LGTM
 from what I can read.

Thanks. I'm happy to explain offline how our helpers (e.g. buildObject)
work, if you want.

iustin


Re: [PATCH master 14/52] Add support for RE patterns to convert constants

2012-01-11 Thread Iustin Pop
On Wed, Jan 11, 2012 at 10:51:56AM +0100, René Nussbaumer wrote:
 On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
  This is a trivial conversion.
  ---
   autotools/convert-constants |   10 ++
   1 files changed, 10 insertions(+), 0 deletions(-)
 
  diff --git a/autotools/convert-constants b/autotools/convert-constants
  index 89f9f00..ae36774 100755
  --- a/autotools/convert-constants
  +++ b/autotools/convert-constants
  @@ -27,8 +27,12 @@ import re
   from ganeti import constants
   from ganeti import compat
 
  +#: Constant name regex
   CONSTANT_RE = re.compile(^[A-Z][A-Z0-9_-]+$)
 
  +#: The type of regex objects
  +RE_TYPE = type(CONSTANT_RE)
  +
 
   def NameRules(name):
    Converts the upper-cased Python name to Haskell camelCase.
  @@ -129,6 +133,12 @@ def ConvertVariable(name, value):
          lines.append(-- | Skipped list/set %s, is not homogeneous % name)
      else:
        lines.append(-- | Skipped list/set %s, cannot convert all elems % 
  name)
  +  elif isinstance(value, RE_TYPE):
  +    tvs = HaskellTypeVal(value.pattern)
 
 According to the doc, this is always a string. Any reason to anyway
 going over HaskellTypeVal?

Oh, just because the HaskellTypeVal will do the quoting correctly for
us; otherwise we'd have to re-do it. So yes, we could use directly
String for the type, but we're using it for the Value.

iustin


Re: [PATCH master 14/52] Add support for RE patterns to convert constants

2012-01-11 Thread René Nussbaumer
On Wed, Jan 11, 2012 at 11:15, Iustin Pop ius...@google.com wrote:
 On Wed, Jan 11, 2012 at 10:51:56AM +0100, René Nussbaumer wrote:
 On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
  This is a trivial conversion.
  ---
   autotools/convert-constants |   10 ++
   1 files changed, 10 insertions(+), 0 deletions(-)
 
  diff --git a/autotools/convert-constants b/autotools/convert-constants
  index 89f9f00..ae36774 100755
  --- a/autotools/convert-constants
  +++ b/autotools/convert-constants
  @@ -27,8 +27,12 @@ import re
   from ganeti import constants
   from ganeti import compat
 
  +#: Constant name regex
   CONSTANT_RE = re.compile(^[A-Z][A-Z0-9_-]+$)
 
  +#: The type of regex objects
  +RE_TYPE = type(CONSTANT_RE)
  +
 
   def NameRules(name):
    Converts the upper-cased Python name to Haskell camelCase.
  @@ -129,6 +133,12 @@ def ConvertVariable(name, value):
          lines.append(-- | Skipped list/set %s, is not homogeneous % name)
      else:
        lines.append(-- | Skipped list/set %s, cannot convert all elems % 
  name)
  +  elif isinstance(value, RE_TYPE):
  +    tvs = HaskellTypeVal(value.pattern)

 According to the doc, this is always a string. Any reason to anyway
 going over HaskellTypeVal?

 Oh, just because the HaskellTypeVal will do the quoting correctly for
 us; otherwise we'd have to re-do it. So yes, we could use directly
 String for the type, but we're using it for the Value.

Fair enough :). LGTM.

René


Re: [PATCH master 3/4] set-mem rapi and tests

2012-01-11 Thread Guido Trotter
Not exactly.

- There is no such thing as min at run time
- As for max it cannot be modified without a reboot.

So it's all true except the last sentence: we just want a way to modify
oper_ram, live. (there is no sense in doing it non-live)

Guido
On Jan 11, 2012 11:02 AM, Iustin Pop ius...@google.com wrote:

 On Tue, Jan 10, 2012 at 06:28:51PM +, Guido Trotter wrote:
  We still need --running-mem: this is meant for ballooning without
 changing
  min and max, which are the only values we currently save.

 Guid, can you confirm this is correct?

 - we have min/max mem, which are config values
 - we have oper_ram, which is the current running memory (between
  min/max)
 - currently gnt-instance modify only allows changing min/max _in the
  config_
 - we want to be able to modify min/max at runtime and also oper_ram at
  runtime

 iustin

  On Jan 10, 2012 3:47 PM, Michael Hanselmann han...@google.com wrote:
 
   Am 10. Januar 2012 12:29 schrieb Iustin Pop ius...@google.com:
Hmm, right. But then I'd make it a different key (e.g. running-mem or
such), but still in instance modify.
  
   Or add a new parameter “--live” to apply the changes right away.
  
   Michael
  



Re: [PATCH master 3/4] set-mem rapi and tests

2012-01-11 Thread Iustin Pop
On Wed, Jan 11, 2012 at 10:58:54AM +, Guido Trotter wrote:
 Not exactly.
 
 - There is no such thing as min at run time

Hmm. Right now (in 2.6) not, but this would have been the value that Xen
(or KVM) is allowed to auto-balloon-down the instance.

But I might be wrong indeed and there's no such memory. Hrmm… Then would
the oper_mem (running_mem) be the minimum for the hypervisor?

I think I misjudged a bit these, I'll have to rethink and see how all
the thinks (capacity, auto-ballooning, etc.) will work.

 - As for max it cannot be modified without a reboot.
 
 So it's all true except the last sentence: we just want a way to modify
 oper_ram, live. (there is no sense in doing it non-live)

Ack. Whether we keep oper_ram (for consistency with instance list) or we
rename it to running_ram it's for decision.

thanks!
iustin

 On Jan 11, 2012 11:02 AM, Iustin Pop ius...@google.com wrote:
 
  On Tue, Jan 10, 2012 at 06:28:51PM +, Guido Trotter wrote:
   We still need --running-mem: this is meant for ballooning without
  changing
   min and max, which are the only values we currently save.
 
  Guid, can you confirm this is correct?
 
  - we have min/max mem, which are config values
  - we have oper_ram, which is the current running memory (between
   min/max)
  - currently gnt-instance modify only allows changing min/max _in the
   config_
  - we want to be able to modify min/max at runtime and also oper_ram at
   runtime
 
  iustin
 
   On Jan 10, 2012 3:47 PM, Michael Hanselmann han...@google.com wrote:
  
Am 10. Januar 2012 12:29 schrieb Iustin Pop ius...@google.com:
 Hmm, right. But then I'd make it a different key (e.g. running-mem or
 such), but still in instance modify.
   
Or add a new parameter “--live” to apply the changes right away.
   
Michael
   
 


Re: [PATCH master 3/4] set-mem rapi and tests

2012-01-11 Thread Guido Trotter
I think auto_ballooning will go as much as it can. I've seen no minimum
there. oper_ram is just the current one, not the minimum, the hypervisor
might go lower. I think that min is the minimum ganeti will guarantee to
be available (never start instances if all of them are nit guaranteed to
have at least their min) oper is just the current runtime and max is the
max an instance can go to.

G
On Jan 11, 2012 12:11 PM, Iustin Pop ius...@google.com wrote:

 On Wed, Jan 11, 2012 at 10:58:54AM +, Guido Trotter wrote:
  Not exactly.
 
  - There is no such thing as min at run time

 Hmm. Right now (in 2.6) not, but this would have been the value that Xen
 (or KVM) is allowed to auto-balloon-down the instance.

 But I might be wrong indeed and there's no such memory. Hrmm… Then would
 the oper_mem (running_mem) be the minimum for the hypervisor?

 I think I misjudged a bit these, I'll have to rethink and see how all
 the thinks (capacity, auto-ballooning, etc.) will work.

  - As for max it cannot be modified without a reboot.
 
  So it's all true except the last sentence: we just want a way to modify
  oper_ram, live. (there is no sense in doing it non-live)

 Ack. Whether we keep oper_ram (for consistency with instance list) or we
 rename it to running_ram it's for decision.

 thanks!
 iustin

  On Jan 11, 2012 11:02 AM, Iustin Pop ius...@google.com wrote:
 
   On Tue, Jan 10, 2012 at 06:28:51PM +, Guido Trotter wrote:
We still need --running-mem: this is meant for ballooning without
   changing
min and max, which are the only values we currently save.
  
   Guid, can you confirm this is correct?
  
   - we have min/max mem, which are config values
   - we have oper_ram, which is the current running memory (between
min/max)
   - currently gnt-instance modify only allows changing min/max _in the
config_
   - we want to be able to modify min/max at runtime and also oper_ram at
runtime
  
   iustin
  
On Jan 10, 2012 3:47 PM, Michael Hanselmann han...@google.com
 wrote:
   
 Am 10. Januar 2012 12:29 schrieb Iustin Pop ius...@google.com:
  Hmm, right. But then I'd make it a different key (e.g.
 running-mem or
  such), but still in instance modify.

 Or add a new parameter “--live” to apply the changes right away.

 Michael

  



Re: [PATCH master 18/52] Update memory/maxmem reading in Rapi backend

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 Recent changes to the instance beparams have replaced memory with
 maxmem in Rapi bulk queries. Until this is either reverted (for
 backwards compat) or we decide to go ahead with only maxmem, we change
 the backend to read both; this only affects the instance down code
 path (where ``oper_ram`` is missing).
 ---
  htools/Ganeti/HTools/Rapi.hs |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

LGTM

René


Re: [PATCH master 19/52] Load cluster ipolicy via Rapi

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 This requires changing from querying the /tags resource to the /info
 resource.
 ---
  htools/Ganeti/HTools/Rapi.hs |   21 -
  1 files changed, 16 insertions(+), 5 deletions(-)

LGTM

René


Re: [PATCH master 20/52] Abstract creation of instance from a spec

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 ---
  htools/Ganeti/HTools/Program/Hspace.hs |   20 
  1 files changed, 12 insertions(+), 8 deletions(-)

LGTM.

René


Re: [PATCH master 22/52] Switch hspace defaults to the cluster policy

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 This changes from the current hardcoded defaults to the cluster
 policy. The command line options now override the defaults from the
 cluster, and the tiered spec mode is always enabled.

 Also fixes a tiny typo in the man page (together with the man page
 updates).
 ---
  htools/Ganeti/HTools/CLI.hs            |    6 ++--
  htools/Ganeti/HTools/Program/Hspace.hs |   29 
  man/hspace.rst                         |   45 ---
  3 files changed, 43 insertions(+), 37 deletions(-)

LGTM

René


Re: [PATCH master 23/52] Fix handling of errors from InstancePolicy.Check...

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 This code raises a configuration error, but we need to transform it
 into a prereq error (or possibly exec error), depending on when we
 call this function.
 ---
  lib/cmdlib.py |   18 +++---
  1 files changed, 15 insertions(+), 3 deletions(-)

LGTM.

René


Re: [okeanos-dev] Re: [RFC master] Implement rbd disk template

2012-01-11 Thread Andrea Spadaccini
Hello Stratos,

 After fighting with a couple of ceph/rbd bugs, today I managed to set
 up a fully operational ceph cluster. Instance creation works, but
 burnin does not: can you please tell me which commit ID did you base
 your tests on? I want to make sure I debug an RBD issue and not
 something unrelated.

 Concerning rbd/rados, we encountered a couple of issues too, while
 testing RBD disks with Ganeti, and so we cherry-picked some commits from
 the master branch, and applied them on v0.39. However, since v0.40 will
 be probably released soon, and will contain a lot of reworked/refactored
 code, I think it would make sense to build and test Ceph from the master
 branch, instead of debugging v0.39 code.

 The most critical issues we encountered and their fixes/commits are
 listed below:

Yes, I have already had #4 (that I fixed for testing by truncating
to 20 chars the image name in ganeti); I am running ceph 0.39 and will
switch to git master if I encounter some of these problems: thanks for
the suggestion.

 I should also note, that we use ext4 (and not btrfs) as the underlying
 filesystem for the OSDs.

Why are you using ext4? For what I understand, it seems like rados is
designed to take advantage of btrfs features.

Also, can you please tell me the Ganeti revision on which you based your patch?

 I will post a detailed patch review after testing it, but before
 please fix the issues that make lint will give you (there are a
 couple of them).

 Constantinos will follow-up with an e-mail about the RBD disk template
 patchset.

Great!

Thanks,
Andrea


[PATCH master] Fix acquisition of node lock in LUInstanceGrowDisk

2012-01-11 Thread Constantinos Venetsanopoulos
Ensure node level locks are recalculated properly
in LUInstanceGrowDisk.

Signed-off-by: Constantinos Venetsanopoulos c...@grnet.gr
---
Hello,

LUInstanceGrowDisk seems to not recalculate locks properly,
leading to this (testing with -t plain):

# gnt-instance grow-disk testgrow 0 1g
Failure: command execution error:
('_LockInstancesNodes helper function called with no nodes to recalculate',)

This patch fixes the problem.

 lib/cmdlib.py |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index 3e86e48..6f90043 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -11242,6 +11242,7 @@ class LUInstanceGrowDisk(LogicalUnit):
 self._ExpandAndLockInstance()
 self.needed_locks[locking.LEVEL_NODE] = []
 self.needed_locks[locking.LEVEL_NODE_RES] = []
+self.recalculate_locks[locking.LEVEL_NODE] = constants.LOCKS_REPLACE
 self.recalculate_locks[locking.LEVEL_NODE_RES] = constants.LOCKS_REPLACE
 
   def DeclareLocks(self, level):
-- 
1.7.2.5



Re: [PATCH master 25/52] Add a new disk-template ipolicy option

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 ---
  lib/cli.py |    7 +++
  1 files changed, 7 insertions(+), 0 deletions(-)

This is to spec for allowed disk templates? I wonder how max/min will
look like here.

The code itself LGTM.

René


Re: [PATCH master 26/52] Move the instance specs options to cli.py

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 Currently these are defined twice, instead of a single time in
 cli.py. Also adds the new disk_templates option to the common block,
 even though it's not yet used.
 ---
  lib/cli.py                |   11 +++
  lib/client/gnt_cluster.py |    8 
  lib/client/gnt_group.py   |    8 
  3 files changed, 11 insertions(+), 16 deletions(-)

Can you add the options to the man-page please?

René


Re: [PATCH master 27/52] Remove extraneous check in policy creation

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 The values are already checked in CreateIPolicy, no need to manually
 check them again.
 ---
  lib/client/gnt_cluster.py |    2 --
  lib/client/gnt_group.py   |    3 ---
  2 files changed, 0 insertions(+), 5 deletions(-)

LGTM.

René


Re: [PATCH master 28/52] Add new disk_templates parameter to instance policy

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 This is a bit more complex patch, as it requires changing the
 assumption that all keys in the policy dict points to values that are
 themselves dicts. Right now we introduce an assumption that any
 non-dicts are lists, we'll see in the future if this holds or whether
 we need more complex type checking (manual, yay Python).

 The patch also does some trivial style changes.
 ---
  lib/bootstrap.py          |    3 +-
  lib/client/gnt_cluster.py |   13 +++--
  lib/client/gnt_group.py   |    1 +
  lib/cmdlib.py             |   64 
  lib/config.py             |   13 +++--
  lib/constants.py          |    4 ++-
  lib/objects.py            |   55 --
  7 files changed, 107 insertions(+), 46 deletions(-)

 diff --git a/lib/bootstrap.py b/lib/bootstrap.py
 index 3e42eae..66c166c 100644
 --- a/lib/bootstrap.py
 +++ b/lib/bootstrap.py
 @@ -421,8 +421,7 @@ def InitCluster(cluster_name, mac_prefix, # pylint: 
 disable=R0913, R0914
     utils.ForceDictType(val, constants.ISPECS_PARAMETER_TYPES)

   objects.NIC.CheckParameterSyntax(nicparams)
 -  full_ipolicy = objects.FillDictOfDicts(constants.IPOLICY_DEFAULTS,
 -                                         ipolicy)
 +  full_ipolicy = objects.FillIPolicy(constants.IPOLICY_DEFAULTS, ipolicy)
   objects.InstancePolicy.CheckParameterSyntax(full_ipolicy)

   if ndparams is not None:
 diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
 index 4af6e22..46f6135 100644
 --- a/lib/client/gnt_cluster.py
 +++ b/lib/client/gnt_cluster.py
 @@ -139,13 +139,16 @@ def InitCluster(opts, args):
     utils.ForceDictType(diskparams[templ], constants.DISK_DT_TYPES)

   # prepare ipolicy dict
 +  ispecs_dts = opts.ispecs_disk_templates # hate long var names
   ipolicy_raw = \
     objects.CreateIPolicyFromOpts(ispecs_mem_size=opts.ispecs_mem_size,
                                   ispecs_cpu_count=opts.ispecs_cpu_count,
                                   ispecs_disk_count=opts.ispecs_disk_count,
                                   ispecs_disk_size=opts.ispecs_disk_size,
 -                                  ispecs_nic_count=opts.ispecs_nic_count)
 -  ipolicy = objects.FillDictOfDicts(constants.IPOLICY_DEFAULTS, ipolicy_raw)
 +                                  ispecs_nic_count=opts.ispecs_nic_count,
 +                                  ispecs_disk_templates=ispecs_dts,
 +                                  fill_all=True)
 +  ipolicy = objects.FillIPolicy(constants.IPOLICY_DEFAULTS, ipolicy_raw)
   for value in ipolicy.values():
     utils.ForceDictType(value, constants.ISPECS_PARAMETER_TYPES)

 @@ -461,6 +464,8 @@ def ShowClusterConfig(opts, args):
   for key in constants.IPOLICY_PARAMETERS:
     ToStdout(  - %s, key)
     _PrintGroupedParams(result[ipolicy][key], roman=opts.roman_integers)
 +  ToStdout(  - enabled disk templates: %s,
 +           utils.CommaJoin(result[ipolicy][constants.ISPECS_DTS]))

   return 0

 @@ -984,12 +989,14 @@ def SetClusterParams(opts, args):
   if ndparams is not None:
     utils.ForceDictType(ndparams, constants.NDS_PARAMETER_TYPES)

 +  ispecs_dts = opts.ispecs_disk_templates
   ipolicy = \
     objects.CreateIPolicyFromOpts(ispecs_mem_size=opts.ispecs_mem_size,
                                   ispecs_cpu_count=opts.ispecs_cpu_count,
                                   ispecs_disk_count=opts.ispecs_disk_count,
                                   ispecs_disk_size=opts.ispecs_disk_size,
 -                                  ispecs_nic_count=opts.ispecs_nic_count)
 +                                  ispecs_nic_count=opts.ispecs_nic_count,
 +                                  ispecs_disk_templates=ispecs_dts)

   mnh = opts.maintain_node_health

 diff --git a/lib/client/gnt_group.py b/lib/client/gnt_group.py
 index f39c6e5..b8d8236 100644
 --- a/lib/client/gnt_group.py
 +++ b/lib/client/gnt_group.py
 @@ -192,6 +192,7 @@ def SetGroupParams(opts, args):
     ispecs_disk_count=opts.ispecs_disk_count,
     ispecs_disk_size=opts.ispecs_disk_size,
     ispecs_nic_count=opts.ispecs_nic_count,
 +    ispecs_disk_templates=opts.ispecs_disk_templates,
     group_ipolicy=True,
     allowed_values=[constants.VALUE_DEFAULT])

 diff --git a/lib/cmdlib.py b/lib/cmdlib.py
 index c520221..d0f5b0a 100644
 --- a/lib/cmdlib.py
 +++ b/lib/cmdlib.py
 @@ -721,6 +721,42 @@ def _GetUpdatedParams(old_params, update_dict,
   return params_copy


 +def _GetUpdatedIPolicy(old_ipolicy, new_ipolicy, group_policy=False):
 +  Return the new version of a instance policy.
 +
 +  @param group_policy: whether this policy applies to a group and thus
 +    we should support removal of policy entries
 +
 +  
 +  use_none = use_default = group_policy
 +  ipolicy = copy.deepcopy(old_ipolicy)
 +  for key, value in new_ipolicy.items():
 +    if key in constants.IPOLICY_PARAMETERS:
 +      utils.ForceDictType(value, constants.ISPECS_PARAMETER_TYPES)
 +  

Re: [RFC master] Implement rbd disk template

2012-01-11 Thread Constantinos Venetsanopoulos

On 01/10/2012 06:17 PM, Andrea Spadaccini wrote:

Hello,


Introduce the rbd disk template, which handles provisioning and
management of instance disks as block devices mapped to rbd volumes
on a RADOS cluster.

Thanks for sending the patch.

After fighting with a couple of ceph/rbd bugs, today I managed to set
up a fully operational ceph cluster. Instance creation works, but
burnin does not: can you please tell me which commit ID did you base
your tests on? I want to make sure I debug an RBD issue and not
something unrelated.

Hello Andrea,

burnin doesn't work for two reasons:

1. Grow disks hits the assertion mentioned on the first mail.
   For that, i just sent a oneline patch that fixes the problem.
2. During failover the iallocator is called and the rbd template
   is not being recognized as a valid DiskTemplate. As mentioned
   by Vangelis in his previous mail, htools should be accordingly
   adjusted. Even so, someone would expect that when running burnin
   with the -n option the iallocator wouldn't be called at all,
   but this doesn't seem to be the case.

The commit ID on top of which the rbd patch is implemented is:
43f52ae3b93e072821d1ba57f03d2099b276ba05

I'll be sending a patch for rbd right after running make lint soon.

Regards,
Constantinos






Re: [PATCH master 25/52] Add a new disk-template ipolicy option

2012-01-11 Thread René Nussbaumer
On Wed, Jan 11, 2012 at 16:13, René Nussbaumer r...@google.com wrote:
 On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 ---
  lib/cli.py |    7 +++
  1 files changed, 7 insertions(+), 0 deletions(-)

 This is to spec for allowed disk templates? I wonder how max/min will
 look like here.

Ah the answer comes later in the series. It's separate.

René


Re: [PATCH master 29/52] Move DiskTemplate definition around

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 This is needed since we'll need the DiskTemplate definition in the
 IPolicy one.
 ---
  htools/Ganeti/HTools/Types.hs |   22 +++---
  1 files changed, 11 insertions(+), 11 deletions(-)

This is just a move further up? LGTM then.

René


Re: [PATCH master 30/52] Read the disk templates part of the ipolicy

2012-01-11 Thread René Nussbaumer
On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
 The default value is badly defined (hardcoded defaults)…
 ---
  htools/Ganeti/HTools/Types.hs |    6 ++
  1 files changed, 6 insertions(+), 0 deletions(-)

LGTM.

René


Re: [PATCH master 26/52] Move the instance specs options to cli.py

2012-01-11 Thread Iustin Pop
On Wed, Jan 11, 2012 at 04:15:01PM +0100, René Nussbaumer wrote:
 On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
  Currently these are defined twice, instead of a single time in
  cli.py. Also adds the new disk_templates option to the common block,
  even though it's not yet used.
  ---
   lib/cli.py                |   11 +++
   lib/client/gnt_cluster.py |    8 
   lib/client/gnt_group.py   |    8 
   3 files changed, 11 insertions(+), 16 deletions(-)
 
 Can you add the options to the man-page please?

Ah, sorry. The previous templates are already in there, here is the
interdiff for my new option, which also does some cleanup to the
previous text:

diff --git a/man/gnt-cluster.rst b/man/gnt-cluster.rst
index 199760b..34d517f 100644
--- a/man/gnt-cluster.rst
+++ b/man/gnt-cluster.rst
@@ -190,6 +190,7 @@ INIT
 | [--specs-disk-size *spec-param*=*value* [,*spec-param*=*value*...]]
 | [--specs-mem-size *spec-param*=*value* [,*spec-param*=*value*...]]
 | [--specs-nic-count *spec-param*=*value* [,*spec-param*=*value*...]]
+| [--specs-disk-templates *template* [,*template*...]]
 | [--disk-state *diskstate*]
 | [--hypervisor-state *hvstate*]
 | {*clustername*}
@@ -371,7 +372,7 @@ must be specified first, followed by a colon and by a 
comma-separated
 list of key-value pairs. These parameters can only be specified at
 cluster and node group level; the cluster-level parameter are inherited
 by the node group at the moment of its creation, and can be further
-modified at node group level using the **gnt-group**(8) command. 
+modified at node group level using the **gnt-group**(8) command.
 
 The following is the list of disk parameters available for the **drbd**
 template, with measurement units specified in square brackets at the end
@@ -483,17 +484,21 @@ The ``-C (--candidate-pool-size)`` option specifies the
 that the master will try to keep as master\_candidates. For more
 details about this role and other node roles, see the ganeti(7).
 
-The ``--specs-..`` options specify instance policy on the cluster. Each
-option can have three values: ``min``, ``max`` and ``std``, which can
-also be modified on group level (except for ``std``, which is defined
-once for the entire cluster). Please note, that ``std`` values are not
-the same as defaults set by ``--beparams``.
-``--specs-cpu-count`` sets the number of VCPUs that can be used by an
-instance.
-``--specs-disk-count`` sets the number of disks
-``--specs-disk-size`` limits the disk size for every disk used
-``--specs-mem-size`` limits the amount of memory available
-``--specs-nic-count`` sets limits on the amount of nics used
+The ``--specs-...`` options specify instance policy on the
+cluster. Except for the ``disk-templates`` option, each option can have
+three values: ``min``, ``max`` and ``std``, which can also be modified
+on group level (except for ``std``, which is defined once for the entire
+cluster). Please note, that ``std`` values are not the same as defaults
+set by ``--beparams``, but they are used for the capacity calculations.
+
+- ``--specs-cpu-count`` limits the number of VCPUs that can be used by an
+  instance.
+- ``--specs-disk-count`` limits the number of disks
+- ``--specs-disk-size`` limits the disk size for every disk used
+- ``--specs-mem-size`` limits the amount of memory available
+- ``--specs-nic-count`` sets limits on the number of NICs used
+- ``--specs-disk-templates`` limits the allowed disk templates (no
+  mix/std/max for this option)
 
 For details about how to use ``--hypervisor-state`` and ``--disk-state``
 have a look at **ganeti**(7).
@@ -565,6 +570,7 @@ MODIFY
 | [--specs-disk-size *spec-param*=*value* [,*spec-param*=*value*...]]
 | [--specs-mem-size *spec-param*=*value* [,*spec-param*=*value*...]]
 | [--specs-nic-count *spec-param*=*value* [,*spec-param*=*value*...]]
+| [--specs-disk-templates *template* [,*template*...]]
 
 
 Modify the options for the cluster.
@@ -601,7 +607,7 @@ The ``-I (--default-iallocator)`` is described in the 
**init**
 command. To clear the default iallocator, just pass an empty string
 ('').
 
-The ``--specs-..`` options are described in the **init** command.
+The ``--specs-...`` options are described in the **init** command.
 
 QUEUE
 ~
diff --git a/man/gnt-group.rst b/man/gnt-group.rst
index 977b3c3..a260e65 100644
--- a/man/gnt-group.rst
+++ b/man/gnt-group.rst
@@ -32,6 +32,7 @@ ADD
 | [--specs-disk-size *spec-param*=*value* [,*spec-param*=*value*...]]
 | [--specs-mem-size *spec-param*=*value* [,*spec-param*=*value*...]]
 | [--specs-nic-count *spec-param*=*value* [,*spec-param*=*value*...]]
+| [--specs-disk-templates *template* [,*template*...]]
 | [--disk-state *diskstate*]
 | [--hypervisor-state *hvstate*]
 | {*group*}
@@ -66,14 +67,8 @@ parameters for the node group; please see the section about
 **gnt-cluster add** in **gnt-cluster**(8) for more information about
 disk parameters
 
-The ``--specs-..`` options specify instance policy on the 

Re: [PATCH master 28/52] Add new disk_templates parameter to instance policy

2012-01-11 Thread Iustin Pop
On Wed, Jan 11, 2012 at 04:32:35PM +0100, René Nussbaumer wrote:
 On Mon, Jan 9, 2012 at 11:58, Iustin Pop ius...@google.com wrote:
  This is a bit more complex patch, as it requires changing the
  assumption that all keys in the policy dict points to values that are
  themselves dicts. Right now we introduce an assumption that any
  non-dicts are lists, we'll see in the future if this holds or whether
  we need more complex type checking (manual, yay Python).
 
  The patch also does some trivial style changes.
  ---
   lib/bootstrap.py          |    3 +-
   lib/client/gnt_cluster.py |   13 +++--
   lib/client/gnt_group.py   |    1 +
   lib/cmdlib.py             |   64 
  
   lib/config.py             |   13 +++--
   lib/constants.py          |    4 ++-
   lib/objects.py            |   55 --
   7 files changed, 107 insertions(+), 46 deletions(-)

  -def FillDictOfDicts(defaults_dict, custom_dict, skip_keys=None):
  -  Run FillDict for each key in dictionary.
  +def FillIPolicy(default_ipolicy, custom_ipolicy, skip_keys=None):
  +  Fills an instance policy with defaults.
 
    
    ret_dict = {}
  -  for key in defaults_dict:
  -    ret_dict[key] = FillDict(defaults_dict[key],
  -                             custom_dict.get(key, {}),
  +  for key in constants.IPOLICY_PARAMETERS:
 
 I wonder if we should have an assert here to verify that
 defaults_ipolicy has all the keys defined that appear in
 constants.IPOLICY_PARAMETERS.
 
 assert len(constants.IPOLICY_PARAMETERS - defaults_ipolicy.keys()) == 0

Either that, or we can allow the code to fail in default_dicts[key]. But
it'd have to be a bit more involved, since IPOLICY_PARAMETERS doesn't
have the DISK_TEMPLATES one.

Let me know if this is what you want:

diff --git a/lib/constants.py b/lib/constants.py
index 08b3cc7..26242db 100644
--- a/lib/constants.py
+++ b/lib/constants.py
@@ -948,6 +948,12 @@ IPOLICY_PARAMETERS = frozenset([
   ISPECS_MAX,
   ISPECS_STD,
   ])
+IPOLICY_ALL_KEYS = frozenset([
+  ISPECS_MIN,
+  ISPECS_MAX,
+  ISPECS_STD,
+  ISPECS_DTS,
+  ])
 
 # Node parameter names
 ND_OOB_PROGRAM = oob_program
diff --git a/lib/objects.py b/lib/objects.py
index a33c720..0cd25c4 100644
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -96,6 +96,7 @@ def FillIPolicy(default_ipolicy, custom_ipolicy, 
skip_keys=None):
   Fills an instance policy with defaults.
 
   
+  assert frozenset(default_ipolicy.keys) == constants.IPOLICY_ALL_KEYS
   ret_dict = {}
   for key in constants.IPOLICY_PARAMETERS:
 ret_dict[key] = FillDict(default_ipolicy[key],

-- 
thanks,
iustin