Re: [PATCH master 28/52] Add new disk_templates parameter to instance policy
On Wed, Jan 11, 2012 at 04:32:35PM +0100, René Nussbaumer wrote: > On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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
Re: [PATCH master 26/52] Move the instance specs options to cli.py
On Wed, Jan 11, 2012 at 04:15:01PM +0100, René Nussbaumer wrote: > On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 c
Re: [PATCH master 30/52] Read the disk templates part of the ipolicy
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 29/52] Move DiskTemplate definition around
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 25/52] Add a new disk-template ipolicy option
On Wed, Jan 11, 2012 at 16:13, René Nussbaumer wrote: > On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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: [RFC master] Implement rbd disk template
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 28/52] Add new disk_templates parameter to instance policy
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 ke
Re: [PATCH master 27/52] Remove extraneous check in policy creation
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 26/52] Move the instance specs options to cli.py
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 25/52] Add a new disk-template ipolicy option
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 24/52] Add a new CLI option type 'list'
On Mon, Jan 9, 2012 at 11:58, Iustin Pop wrote: > This simply splits the value in the option parser, instead of needing > to do it in the client code. > --- > lib/cli.py | 14 ++ > 1 files changed, 14 insertions(+), 0 deletions(-) LGTM. Seems like there's just the append feature which is not the same. René
[PATCH master] Fix acquisition of node lock in LUInstanceGrowDisk
Ensure node level locks are recalculated properly in LUInstanceGrowDisk. Signed-off-by: Constantinos Venetsanopoulos --- 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: [okeanos-dev] Re: [RFC master] Implement rbd disk template
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
Re: [PATCH master 23/52] Fix handling of errors from InstancePolicy.Check...
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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: [PATCH master 22/52] Switch hspace defaults to the cluster policy
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 21/52] Add a helper function converting ispecs to rspecs
On Mon, Jan 9, 2012 at 11:58, Iustin Pop wrote: > The rspec type is simpler than the ispec one; most likely it should be > deprecated later. > --- > htools/Ganeti/HTools/Types.hs | 8 > 1 files changed, 8 insertions(+), 0 deletions(-) LGTM René
Re: [PATCH master 20/52] Abstract creation of instance from a spec
On Mon, Jan 9, 2012 at 11:58, Iustin Pop wrote: > --- > htools/Ganeti/HTools/Program/Hspace.hs | 20 > 1 files changed, 12 insertions(+), 8 deletions(-) LGTM. René
Re: [PATCH master 19/52] Load cluster ipolicy via Rapi
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 18/52] Update memory/maxmem reading in Rapi backend
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 3/4] set-mem rapi and tests
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" 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" 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" > wrote: > > > > > > > > > Am 10. Januar 2012 12:29 schrieb Iustin Pop : > > > > > > 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
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" 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" wrote: > > > > > > > Am 10. Januar 2012 12:29 schrieb Iustin Pop : > > > > > 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
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" 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" wrote: > > > > > Am 10. Januar 2012 12:29 schrieb Iustin Pop : > > > > 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 14/52] Add support for RE patterns to convert constants
On Wed, Jan 11, 2012 at 11:15, Iustin Pop wrote: > On Wed, Jan 11, 2012 at 10:51:56AM +0100, René Nussbaumer wrote: >> On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 14/52] Add support for RE patterns to convert constants
On Wed, Jan 11, 2012 at 10:51:56AM +0100, René Nussbaumer wrote: > On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 09/52] Add object definitions for the ispec and ipolicy
On Wed, Jan 11, 2012 at 10:17:38AM +0100, René Nussbaumer wrote: > On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 3/4] set-mem rapi and tests
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" wrote: > > > Am 10. Januar 2012 12:29 schrieb Iustin Pop : > > > 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 17/52] Load cluster ipolicy via Luxi
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 16/52] Extend ClusterData with the cluster instance policy
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 15/52] Add default ipolicy declarations
On Mon, Jan 9, 2012 at 11:58, Iustin Pop wrote: > --- > htools/Ganeti/HTools/Types.hs | 35 +++ > 1 files changed, 35 insertions(+), 0 deletions(-) LGTM. René
Re: [PATCH master 14/52] Add support for RE patterns to convert constants
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 13/52] Add support for lists/frozensets in convert-constants
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 12/52] Add support for tuples in convert-constants
On Mon, Jan 9, 2012 at 11:58, Iustin Pop wrote: > --- > autotools/convert-constants | 11 +++ > 1 files changed, 11 insertions(+), 0 deletions(-) LGTM. René
Re: [PATCH master 11/52] More improvements to convert-constants
On Mon, Jan 9, 2012 at 11:58, Iustin Pop wrote: > This prepares for tuple and other conversions. > --- > autotools/convert-constants | 41 ++--- > 1 files changed, 26 insertions(+), 15 deletions(-) LGTM. René
Re: [PATCH master 10/52] Improve convert-constants to handle dictionaries
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 09/52] Add object definitions for the ispec and ipolicy
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 08/52] Stop exporting JSON functionality from Utils.hs
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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 07/52] More reshuffling of code
On Mon, Jan 9, 2012 at 11:58, Iustin Pop 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é