ganeti-github.git
7 years agoBump up version for 2.7.0~beta3 devel-2.7 github/devel-2.7 v2.7.0beta3
Guido Trotter [Mon, 22 Apr 2013 09:05:07 +0000 (11:05 +0200)]
Bump up version for 2.7.0~beta3

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

7 years agoUpdate NEWS for 2.7.0~beta3
Guido Trotter [Thu, 18 Apr 2013 13:53:11 +0000 (15:53 +0200)]
Update NEWS for 2.7.0~beta3

- Most entries for beta1 are moved to the top level
- The beta1 to beta2 delta is left

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

7 years agoAllow proper cleanup of partially created disks
Michele Tartara [Tue, 16 Apr 2013 12:56:39 +0000 (12:56 +0000)]
Allow proper cleanup of partially created disks

During the creation of an instance, if the creation of disks fails, some
partially created disks might remain lying around. There is an already
implemented cleanup procedure, but it requires the disks to be listed in the
disks_created variable, which is updated after the creation of the disk itself
to prevent it from removing disks already existing previously.

This patch introduces a better tracking of partially created disks, so that
they can be removed without risking removing already existing ones.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoBetter wording of a documentation sentence
Michele Tartara [Thu, 18 Apr 2013 08:26:34 +0000 (10:26 +0200)]
Better wording of a documentation sentence

Specify that the need to manually modify the metavg only applies to
clusters with DRBD enabled, and is not always required.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoSpecify behaviour of vg-name change in gnt-cluster manual
Michele Tartara [Thu, 18 Apr 2013 08:01:17 +0000 (10:01 +0200)]
Specify behaviour of vg-name change in gnt-cluster manual

The current behaviour of a volume group name change after the cluster
has been initialized can be somewhat unclear. This patch specifies it
more esplicitly in the man page.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Klaus Aehlig <aehlig@google.com>

7 years agoSet the default metavg to be equal to the cluster name
Michele Tartara [Wed, 17 Apr 2013 15:32:00 +0000 (17:32 +0200)]
Set the default metavg to be equal to the cluster name

The default metavg was always set to be the constant value "xenvg".
This is OK for most cases, but if the cluster is initialized with
a default name through the --vg-name option, the metavg should change
as well.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agousers setup: add masterd to confd group
Guido Trotter [Tue, 16 Apr 2013 15:06:29 +0000 (08:06 -0700)]
users setup: add masterd to confd group

Without this masterd fails saving the config, because it can't change
the group on the file.

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoMove HooksMaster out of the mcpu module
Guido Trotter [Sun, 14 Apr 2013 15:51:08 +0000 (08:51 -0700)]
Move HooksMaster out of the mcpu module

We need to do this, so that backend.py doesn't need to import mcpu, and
thus indirectly cmdlib. This reduces the size of the node daemon by
about half, which is very important as it is pinned in memory.

This solves Issue 419.

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

7 years agoUse KB as the unit for LVM PE size
Aaron Knister [Mon, 15 Apr 2013 16:26:36 +0000 (12:26 -0400)]
Use KB as the unit for LVM PE size

LogicalVolume.Attach in bdev.py is calling "lvs" and specifying a unit of
Megabytes that is then converted to an integer resulting in the value 0 for
small sizes.

This patch makes Ganeti use KB for the unit instead of MB.

Fixes Issue 347.

Signed-off-by: Aaron Knister <aaron.knister@gmail.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

7 years agoImprove RAPI documentation for command line usage
Michele Tartara [Fri, 12 Apr 2013 08:47:06 +0000 (08:47 +0000)]
Improve RAPI documentation for command line usage

Provide some more insights about how to use the RAPI from the command line
through the "curl" command.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoMore info on using the RAPI
Michele Tartara [Tue, 26 Mar 2013 16:21:43 +0000 (16:21 +0000)]
More info on using the RAPI

Add some additional information about how to use/test the RAPI,
including how to specify authentication credentials.

Cherry picked from 681f4f7bb95a2b467f917c84a68ccf9aa7665763 on master.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix burnin install path
Guido Trotter [Thu, 11 Apr 2013 20:23:36 +0000 (16:23 -0400)]
Fix burnin install path

Due to a bug in 8e55e20f burnin's installation path got changed.
Moving tools/burnin to the right makefile target reverts this (while
preserving the value of the original commit).

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoFix format of the NEWS file
Klaus Aehlig [Thu, 11 Apr 2013 16:29:55 +0000 (18:29 +0200)]
Fix format of the NEWS file

Signed-off-by: Klaus Aehlig <aehlig@google.com>
Reviewed-by: Bernardo Dal Seno <bdalseno@google.com>

7 years agoNEWS: Add news entry for the hail disk policy fix
Klaus Aehlig [Thu, 11 Apr 2013 11:37:48 +0000 (13:37 +0200)]
NEWS: Add news entry for the hail disk policy fix

hail now correctly applies the instance disk policies, which
leads to differnt allocations.

Signed-off-by: Klaus Aehlig <aehlig@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoAdd shelltests verifying hail applies disk ipolicy per disk
Klaus Aehlig [Tue, 9 Apr 2013 09:24:09 +0000 (11:24 +0200)]
Add shelltests verifying hail applies disk ipolicy per disk

We test on two minimal examples, a positive and a negative one, where
the possibility to allocate a node depends on whether the disk policy
is checked on a per-disk level or on the total amount of disk space
requested.

Signed-off-by: Klaus Aehlig <aehlig@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoMake the disks parameter available to the constructor
Klaus Aehlig [Tue, 9 Apr 2013 13:52:09 +0000 (15:52 +0200)]
Make the disks parameter available to the constructor

In that way, tools building on Instance will benefit from the corrected
verification semantics of the instance policy on disk space.

Signed-off-by: Klaus Aehlig <aehlig@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoVerify individual disks in Instance
Klaus Aehlig [Tue, 9 Apr 2013 12:29:59 +0000 (14:29 +0200)]
Verify individual disks in Instance

Instance policy on disks is specified on a per-disk basis. So extend
the instance description by the sizes of the individual disks and modify
the instance policy verification to correctly check individual disks.

Signed-off-by: Klaus Aehlig <aehlig@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoCompatibility fix for GHC 7
Michele Tartara [Thu, 11 Apr 2013 11:33:52 +0000 (13:33 +0200)]
Compatibility fix for GHC 7

Importing an instance, like JSON, automatically allows the names of all
its functions to be used.

If one of this functions (namely showJSON) is imported separately, GHC 6
is fine with that, but GHC 7 emits a warning (therefore, the compilation of
Ganeti fails because it requires no warnings).

This patch fixes the build for GHC 7 and has no impact on GHC 6.

Note that the same does not apply to the import of readJSON, because that
function is actually used in the same file, so it must be imported separately.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoProperly update iv_name of disks while changing templates
Michele Tartara [Wed, 10 Apr 2013 14:36:44 +0000 (14:36 +0000)]
Properly update iv_name of disks while changing templates

Trasforming the disk of an instance from DRBD to plain did not properly
update the iv_name of disks, leaving it to None.

This commit modifies _ConvertDrbdToPlain to properly set the iv_name
variables.

An analogous fix is not required in the opposite function,
_ConvertPlainToDrbd, because it uses _GenerateDiskTemplate, that already
takes care of doing that.

Fixes Issue 403.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoLimit the size of networks to /16
Helga Velroyen [Wed, 10 Apr 2013 12:06:19 +0000 (14:06 +0200)]
Limit the size of networks to /16

This patch introduces an upper limit to the size of the networks that can
be created.

Signed-off-by: Helga Velroyen <helgav@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoCheck minimum size of networks on creation
Helga Velroyen [Mon, 21 Jan 2013 15:20:34 +0000 (16:20 +0100)]
Check minimum size of networks on creation

When creating a network, so far no size constraints were checked.
We now limit the size of a network to a /30 or bigger, although
tecnically, the ipaddr library supports even /32 networks.

Signed-off-by: Helga Velroyen <helgav@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix job queue directory permission problems
Michele Tartara [Mon, 8 Apr 2013 12:33:37 +0000 (12:33 +0000)]
Fix job queue directory permission problems

If split users are used, the queue directory could only be accessed
by masterd, but also confd needs to be able to read it, e.g. when it
is queried as part of "gnt-job list"

This commit fixes the permissions in such a way to allow proper access rights.

Fixes Issue 406.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoThe disk size of a diskless instance is 0, not None
Klaus Aehlig [Mon, 8 Apr 2013 14:35:09 +0000 (16:35 +0200)]
The disk size of a diskless instance is 0, not None

For diskless instances it is still reasonable to sum up the disk usage
of all the (zero) disks, resulting in the empty sum. This uniformity
also has the advantage that iallocators (like hail) do not have to do
any special considerations for diskless instances.

Signed-off-by: Klaus Aehlig <aehlig@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoPostpone non-urgent TODO from 2.7 to 2.9
Michele Tartara [Tue, 9 Apr 2013 13:56:13 +0000 (15:56 +0200)]
Postpone non-urgent TODO from 2.7 to 2.9

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoAdd tool for creating users and groups
Michele Tartara [Tue, 9 Apr 2013 13:12:02 +0000 (13:12 +0000)]
Add tool for creating users and groups

When executed in split user mode, Ganeti requires a specific set of users
and groups to be created in the appropriate way.

This commit introduces a tool that takes care of that automatically.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoBackwards compatibility fix for Lucid
Michele Tartara [Mon, 8 Apr 2013 09:15:30 +0000 (09:15 +0000)]
Backwards compatibility fix for Lucid

The code introduced by the previous commit triggered a possible library
conflict in Ubuntu Lucid.

This patch introduces an equivalent but more widely acceptable version of
the same code.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoProperly export errors while reading job list
Michele Tartara [Thu, 4 Apr 2013 16:49:49 +0000 (18:49 +0200)]
Properly export errors while reading job list

In case of problems while reading the job list from disk (such as permission
errors) confd would silently fail, writing a warning on the log file but
sending an empty list and no error message to the client.

Also, tests have been updated in accordance to the new interface of the modified
functions.

This commit fixes this problem [Issue 405].

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix typo and improve comment
Michele Tartara [Thu, 4 Apr 2013 09:13:21 +0000 (11:13 +0200)]
Fix typo and improve comment

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix a bug in the Runtime tests
Michele Tartara [Fri, 5 Apr 2013 13:42:31 +0000 (13:42 +0000)]
Fix a bug in the Runtime tests

A group name was erroneusly inserted in the list of user names.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoRestrict instance move to templates based on local files
Klaus Aehlig [Wed, 3 Apr 2013 15:16:09 +0000 (17:16 +0200)]
Restrict instance move to templates based on local files

Moving an instance is done by copying over the disks. Restrict
this to disk templates that are copyable. This avoids accidental
use on, e.g., the sharedfile template.

Signed-off-by: Klaus Aehlig <aehlig@google.com>
Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Bernardo Dal Seno <bdalseno@google.com>

7 years agoIntroduce a constant for the copyable disk templates
Klaus Aehlig [Wed, 3 Apr 2013 15:16:00 +0000 (17:16 +0200)]
Introduce a constant for the copyable disk templates

This list contains the disk templates suitable for moving
an instance by copying the files. A requirement is that
they're not accessed externally or shared between nodes;
in particular, sharedfile is not suitable.

Signed-off-by: Klaus Aehlig <aehlig@google.com>
Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Bernardo Dal Seno <bdalseno@google.com>

7 years agoDo not _RemoveDisks after failed _CreateDisks
Klaus Aehlig [Wed, 3 Apr 2013 09:21:27 +0000 (11:21 +0200)]
Do not _RemoveDisks after failed _CreateDisks

Now that _CreateDisks cleans up after itself in case of failure,
do not clean up at call sites, as there we have to overapproximate
thus potentially causing data loss.

Signed-off-by: Klaus Aehlig <aehlig@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoMake _CreateDisk clean up partially created disks on failure
Klaus Aehlig [Wed, 3 Apr 2013 09:21:20 +0000 (11:21 +0200)]
Make _CreateDisk clean up partially created disks on failure

_CreateDisk used to just throw an exception if _CreateBlockDev failed
leaving the caller in the state that some disks were created, without
precise knowledge which. Usually, the clean up then overapproximated
by removing all disks of the instance. This can cause data loss in case
the disk creation failed because of a file being already in the place
where the disk is to be created.

With this patch, upon failure, _CreateDisk removes the disks that were
successfully created, leaving a well-defined state.

Signed-off-by: Klaus Aehlig <aehlig@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix typo in an error message
Michele Tartara [Wed, 3 Apr 2013 09:07:59 +0000 (11:07 +0200)]
Fix typo in an error message

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoBump up version for 2.7.0~beta2 v2.7.0beta2
Guido Trotter [Tue, 2 Apr 2013 12:18:50 +0000 (14:18 +0200)]
Bump up version for 2.7.0~beta2

Also update the release date for beta2

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Bernardo Dal Seno <bdalseno@google.com>

7 years agocheck-news: fix behavior on invalid date line
Guido Trotter [Tue, 2 Apr 2013 12:38:18 +0000 (14:38 +0200)]
check-news: fix behavior on invalid date line

Before:
$ ./autotools/check-news < ./NEWS
Traceback (most recent call last):
  File "./autotools/check-news", line 150, in <module>
    main()
  File "./autotools/check-news", line 120, in main
    parsed_ts = time.mktime(time.strptime(m.group("date"), "%d %b %Y"))
AttributeError: 'NoneType' object has no attribute 'group'

After:
$ ./autotools/check-news < ./NEWS
Line 8: Invalid release line

This happened because the error function just updates an error log,
rather than bailing out immediately.

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

7 years agoNews updates for 2.7.0 beta2
Guido Trotter [Tue, 12 Mar 2013 17:06:37 +0000 (18:06 +0100)]
News updates for 2.7.0 beta2

Mention most relevant updates and user visible changes and fixes.

These entries will only be maintained during the beta/rc series, but
will then be replaced with a single "2.7.0" entry when we will release
2.7.

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agocfgupgrade: Fix error messages
Bernardo Dal Seno [Wed, 20 Mar 2013 13:24:49 +0000 (14:24 +0100)]
cfgupgrade: Fix error messages

Messages were inconsistent. You could get both an error and a message
saying that everything was fine.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoUpdate "FIXME" string in RAPI
Michele Tartara [Wed, 20 Mar 2013 11:16:47 +0000 (12:16 +0100)]
Update "FIXME" string in RAPI

We are not ready for this change yet. Let's push it to 2.8.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agorapi client: add target_node to migrate instance
Daniel Krambrock [Tue, 12 Mar 2013 17:17:24 +0000 (18:17 +0100)]
rapi client: add target_node to migrate instance

This allows migrating to any node, as it is already possible for
failover, when instances are externally mirrored.

Signed-off-by: Daniel Krambrock <danielk_lists@z9d.de>
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoMake diskless instances externally mirrored
Guido Trotter [Tue, 12 Mar 2013 15:07:04 +0000 (16:07 +0100)]
Make diskless instances externally mirrored

This addresses Issue 237.

Mirroring no disk is a no-op. As such we can treat them like mirrored
instances, since the data they need (none) will be present on all nodes.

This is definitely enough to failover or migrate instances with a manual
target node. Further work might be needed for support in hail.

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

7 years agoFix migrate/failover -n for ext mirror storage
Guido Trotter [Tue, 12 Mar 2013 15:24:44 +0000 (16:24 +0100)]
Fix migrate/failover -n for ext mirror storage

This fixes issue 396.

- Fix a wrong comment that mentions drbd8 when actually the code acts
  only on externally mirrored instances.
- Fix a wrong assert that requires failover/migrate to acquire the NAL
  on externally mirrored instances: this is the case only when a
  iallocator is used, not if a node is manually specified.

Extra qa should be added, but this can only be done if an externally
mirrored instance or a diskless instance is tested. It is easier to add
it on the master branch, where we have diskless instances support.

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

7 years agoAdd QA for instance creation with policy violation
Bernardo Dal Seno [Wed, 27 Feb 2013 10:47:02 +0000 (11:47 +0100)]
Add QA for instance creation with policy violation

When instance policy is violated, creation fails.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoAdd QA for policy-instance interactions
Bernardo Dal Seno [Fri, 22 Feb 2013 18:07:33 +0000 (19:07 +0100)]
Add QA for policy-instance interactions

Violations on policy changes are checked.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoAdd QA for cluster policies
Bernardo Dal Seno [Thu, 21 Feb 2013 17:58:20 +0000 (18:58 +0100)]
Add QA for cluster policies

qa_cluster.TestClusterSetISpecs() is exported as it will be used in future
tests.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoUnit tests for objects.InstancePolicy + a fix
Bernardo Dal Seno [Tue, 19 Feb 2013 21:13:27 +0000 (22:13 +0100)]
Unit tests for objects.InstancePolicy + a fix

Tests for:
  objects.InstancePolicy.CheckParameterSyntax()
  objects.InstancePolicy.CheckDiskTemplates()
  objects.InstancePolicy.CheckISpecSyntax()

Instance policies with an empty disk-template list now are reported.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoUnit tests for objects.FillIPolicy() + small fix
Bernardo Dal Seno [Tue, 19 Feb 2013 13:56:05 +0000 (14:56 +0100)]
Unit tests for objects.FillIPolicy() + small fix

IPOLICY_DEFAULTS is now a legal policy (the disk-templates entry was a set
instead of a list, before).

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix upgrade of policy in objects.Cluster
Bernardo Dal Seno [Wed, 27 Feb 2013 17:50:17 +0000 (18:50 +0100)]
Fix upgrade of policy in objects.Cluster

Unknown elements were silently removed on startup. This means that a
software upgrade could result in lost configuration information if
cfgupgrade wasn't run promptly.

Added unit test for Cluster.UpgradeConfig() to cover this case.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix instance policy checks for default back-end parameters
Bernardo Dal Seno [Thu, 7 Mar 2013 02:14:56 +0000 (03:14 +0100)]
Fix instance policy checks for default back-end parameters

Policy violations of back-end parameters that used the cluster default
value were not reported in cluster-verify.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix restoring default instance specs in group policies
Bernardo Dal Seno [Mon, 25 Feb 2013 13:16:40 +0000 (14:16 +0100)]
Fix restoring default instance specs in group policies

"default" was not accepted as a valid input value for instance specs in
group policies, due to a bug introduced in 2cc673a3e (and released with
2.6.0). Added QA for this and another similar case.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoUnit tests for cmdlib._GetUpdatedIPolicy()
Bernardo Dal Seno [Mon, 18 Feb 2013 17:02:06 +0000 (18:02 +0100)]
Unit tests for cmdlib._GetUpdatedIPolicy()

Not 100% coverage, though.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix policy check for disk templates
Bernardo Dal Seno [Thu, 7 Mar 2013 00:46:14 +0000 (01:46 +0100)]
Fix policy check for disk templates

Instance disk template is checked against the policy, and diskless
instances aren't checked for the number of disks.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix merge 8e09e801 that resulted in duplicated code
Bernardo Dal Seno [Thu, 7 Mar 2013 00:34:35 +0000 (01:34 +0100)]
Fix merge 8e09e801 that resulted in duplicated code

A fragment in LUInstanceCreate.CheckPrereq() removed in commit ba147ff8 was
reintroduced in merge 8e09e801 due to a change in df28c49b.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoGanetiRapiClient: fix the no_remember option
Daniel Krambrock [Mon, 11 Mar 2013 13:05:21 +0000 (14:05 +0100)]
GanetiRapiClient: fix the no_remember option

There was a typo which prevented the correct option from being passed to
RAPI

Signed-off-by: Daniel Krambrock <danielk_lists@z9d.de>
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoChange hbal behaviour in case of early exit
Iustin Pop [Wed, 6 Mar 2013 09:58:53 +0000 (10:58 +0100)]
Change hbal behaviour in case of early exit

Currently, hbal exits with status 1 if early exit is requested, even
when all jobs are successful. This is counter-intuitive behaviour, so
let's fix it (Issue 386).

Note that the man page had conflicting information already, so it's a
good thing to clean this up.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix build/sphinx_ext.py with tuple defaults for op params
Iustin Pop [Wed, 6 Mar 2013 11:04:12 +0000 (12:04 +0100)]
Fix build/sphinx_ext.py with tuple defaults for op params

When an OpCode's parameter has a tuple as default value, this code
will break:

  buf.write("defaults to ``%s``" % default)

The patch fixes this and other potential cases by always passing a
tuple to '%'.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

7 years agoFix bug in man build rule
Iustin Pop [Wed, 6 Mar 2013 09:43:51 +0000 (10:43 +0100)]
Fix bug in man build rule

Currently the man page build rule is run without ‘-e’, so the actual
checks in there are just warnings (check-man-warnings,
check-man-dashes).

The patch fixes this and also fixes the man pages which made me see the
problem.

Additionally, check-man-dashes is now verbose, otherwise it's hard to
find out actually where in the page the error is.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

7 years agoFix hscolour style sheet building
Iustin Pop [Tue, 5 Mar 2013 17:13:14 +0000 (18:13 +0100)]
Fix hscolour style sheet building

Currently we only symlink this into two hard-coded directories, which
breaks the source code viewing for all the others.

The patch replaces this with automatic building; as playing with
relative symlinks is non-trivial, I just generate it many times (the
file is small enough (<250) that it shouldn't matter, size-wise, and
it takes 2ms to do it).

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

7 years agoUpdate required pylint version
Iustin Pop [Tue, 5 Mar 2013 15:00:50 +0000 (16:00 +0100)]
Update required pylint version

This should help a lot of users, since pylint 0.21.1 was quite an old
version…

The tree is "clean" w.r.t. the new pylint version.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

7 years agoConstants.hs.in: improve Haddock markup in the template
Iustin Pop [Tue, 5 Mar 2013 13:12:42 +0000 (14:12 +0100)]
Constants.hs.in: improve Haddock markup in the template

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

7 years agoconvert-constants: generate better Haddock markup
Iustin Pop [Tue, 5 Mar 2013 13:11:24 +0000 (14:11 +0100)]
convert-constants: generate better Haddock markup

Currently the Python source constants are used as-is, which is not
nice on the eyes (especially for unqualified names, it was hard to
separate them visually).

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

7 years agoExpand Haddock to run over test files as well
Iustin Pop [Tue, 5 Mar 2013 13:07:50 +0000 (14:07 +0100)]
Expand Haddock to run over test files as well

This patch does multiple enhancements to the way we build the Haddock
docs, motivated by the fact that if we don't run Haddock over test
files as well, bad formatting can be submitted and will accumulate
over time (without any checks).

The patch does:

- replace manual built ALL_APIDOC_HS_DIRS with automatically built one
  (from HS_DIRS)
- change Haddock so that it runs from the top directory (instead of
  from src/)
- change HsColour target file to be built via bash parameter
  substitution, rather than sed (I don't know how to do it in one go,
  so I use 2 intermediate variables)
- change 'hs-apidoc' target so that it depends on the real target
  file; in case no source file has been modified, running 'make
  hs-apidoc' twice will not result in two runs
- run HsColour/Haddock under en_US.UTF-8 locale, otherwise they can't
  parse correctly the Unicode chars in the test files

Additionally, wrong formatting (oops) in a test file has been
corrected.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

7 years agoAdd export lists for files which didn't have them
Iustin Pop [Mon, 4 Mar 2013 15:39:00 +0000 (16:39 +0100)]
Add export lists for files which didn't have them

Two test helper files didn't have export lists (lost during the split
of tests). This patch finally reintroduces them, to hopefully make the
export list saner and with fewer changes for purely-internal changes.

Additionally, a few missing docstrings are added as well.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

7 years agoFix Haskell compatibility tests with disabled file storage
Iustin Pop [Mon, 4 Mar 2013 12:32:22 +0000 (13:32 +0100)]
Fix Haskell compatibility tests with disabled file storage

When file storage is disabled at ./configure time, we shouldn't pass
opcodes containing DTFile/DTSharedFile/StorageFile to Python for
validation, as they will fail.

This patch implements this by simply tweaking the Arbitrary instances
for DiskTemplate and StorageType (which IMHO is a nice and clean
way!), and also fixing the generation of arbitrary IPolicies to use
the correct 'allDiskTemplates' list (otherwise we'd loop forever
trying to generate a list of all templates).

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

7 years agoFix QA with disabled file storage
Iustin Pop [Mon, 4 Mar 2013 12:07:25 +0000 (13:07 +0100)]
Fix QA with disabled file storage

Currently QA tries to use “gnt-node list-storage” with all storage
types, but when file storage is disabled this will fail.

The patch changes it so that if file storage is disabled, we test that
indeed fails, and otherwise we include it in the regular tests.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

7 years agoFix convert-constants handling of booleans
Iustin Pop [Mon, 4 Mar 2013 10:54:06 +0000 (11:54 +0100)]
Fix convert-constants handling of booleans

It turns out that, in Python, booleans are also integers. So they fall
under the “isinstance(value, int)” case, resulting in all enable*
constants being integers in Haskell, which is not nice, even though
we're not using them directly today.

Patch simply adds a special casing for booleans, before integers.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix handling of disabled (shared) file storage
Iustin Pop [Mon, 4 Mar 2013 10:44:10 +0000 (11:44 +0100)]
Fix handling of disabled (shared) file storage

The vcluster changes broke the disabling of file storage; we can
workaround by (manually) skipping the virtualisation of file storage
paths if they are not enabled.

Note that tests/QA are still broken with disabled file storage; this
patch only fixes production code.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix low verbosity levels in htools
Iustin Pop [Mon, 4 Mar 2013 10:24:51 +0000 (11:24 +0100)]
Fix low verbosity levels in htools

In a few cases, we tested the verbosity level for (== 0), instead of
higher/lower than a certain value. If the user passes multiple
"--quiet" options, this can result in negative verbosity levels, which
behave like "extra verbosity".

Sigh for plain data types as opposed to proper verbosity levels…

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix improperly formatted docstring
Michele Tartara [Wed, 6 Feb 2013 09:16:01 +0000 (09:16 +0000)]
Fix improperly formatted docstring

Change the docstring of chompPrefix to prevent the error
"doc comment parse failed" that was raised by some version of
haddock while generating the documentation for this function.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>
(cherry picked from commit 94042ae4f2f6983a04d1ff2c99144330562b6301)

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Bernardo Dal Seno <bdalseno@google.com>

7 years agoAllow iallocator to work without LVM storage
Iustin Pop [Fri, 1 Mar 2013 11:45:38 +0000 (12:45 +0100)]
Allow iallocator to work without LVM storage

Currently, the iallocator interface requires LVM storage, due to the
way it computes the node storage information.

By changing the code to understand that GetVGName() can return None,
and by setting the disk_total/disk_free node parameters to the value
zero, we can workaround this issue until proper multi-storage
reporting is implemented.

The hail iallocator plugin works without changes once this is
implemented, as it already handles non-LVM storage as "foreign"
storage (with unlimited capacity).

The patch also changes a bit how the parameters are retrieved and
used; hopefully this is a bit safer and more readable.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoAllow rpc.MakeLegacyNodeInfo to parse non-LVM results
Iustin Pop [Fri, 1 Mar 2013 11:23:23 +0000 (12:23 +0100)]
Allow rpc.MakeLegacyNodeInfo to parse non-LVM results

'MakeLegacyNodeInfo' is not the best place for this, but we'd have to
duplicate it if we wanted a LVM-less version, so the easiest is to add
an optional parameter that allows it to accept/skip LVM-less results.

It still requires at most one VG result, so its behaviour isn't
changed in this respect.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix LUTestAllocator with instance alloc
Iustin Pop [Fri, 1 Mar 2013 09:59:55 +0000 (10:59 +0100)]
Fix LUTestAllocator with instance alloc

This is similar to commit 8775e62a; the addition of node_whitelist
broke this LU as well.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Bernardo Dal Seno <bdalseno@google.com>

7 years agoFix confd issue regarding --no-lvm-storage
Dimitris Aragiorgis [Thu, 28 Feb 2013 01:29:33 +0000 (03:29 +0200)]
Fix confd issue regarding --no-lvm-storage

If cluster is initialized with --no-lvm-storage then volume_group_name
does not exist in config.data. Thus we must define it as optional in
confd.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
[iustin@google.com: fixed Haskell RPC definition]
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

7 years agoFix networks in _PrepareNicModifications()
Dimitris Aragiorgis [Tue, 26 Feb 2013 20:28:42 +0000 (22:28 +0200)]
Fix networks in _PrepareNicModifications()

Passing --net 0:add,ip=5.5.5.5 failed due to a reference
of a non initialized variable (new_net_obj). Reorder the checks
and add some comments to be readable.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix sphinx label namespace
Iustin Pop [Thu, 28 Feb 2013 16:40:39 +0000 (17:40 +0100)]
Fix sphinx label namespace

Thanks Michael for pointing out the error!

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

7 years agoClarify use of move-instance with self-signed certificates
Iustin Pop [Thu, 28 Feb 2013 15:32:11 +0000 (16:32 +0100)]
Clarify use of move-instance with self-signed certificates

The document doesn't mention which certificates need to be passed as
ca-file, so let's try to clarify that.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

7 years agoRemove early returns in network LUs
Dimitris Aragiorgis [Thu, 28 Feb 2013 13:55:03 +0000 (15:55 +0200)]
Remove early returns in network LUs

Remove any early returns in LUNetworkDisconnect/LUNetworkConnect
and replace them with if-else statements.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

7 years agoFix HooksDict() in case of no tags
Dimitris Aragiorgis [Tue, 26 Feb 2013 01:39:28 +0000 (03:39 +0200)]
Fix HooksDict() in case of no tags

In this method self.tags might be None and cannot be used
in join(). Use GetTags instead.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoAdd networks to _AllIDs()
Dimitris Aragiorgis [Tue, 26 Feb 2013 01:39:27 +0000 (03:39 +0200)]
Add networks to _AllIDs()

networks config objects have UUIDs and thus should be included
in _AllUUIDObjects().

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix locking in LUNetworkConnect()
Dimitris Aragiorgis [Tue, 26 Feb 2013 01:39:26 +0000 (03:39 +0200)]
Fix locking in LUNetworkConnect()

Locks for group instances are aquired only if conflicts are checked.
To this end we must _CheckNodeGroupInstances() only then otherwise
this check will always fail (owned_instances will be []).

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix networks in LUInstanceSetParams()
Dimitris Aragiorgis [Tue, 26 Feb 2013 01:39:25 +0000 (03:39 +0200)]
Fix networks in LUInstanceSetParams()

Params passed in _CreateNewNic() are not yet evaluated and include
the value passed by user for the network. A lookup must be done first
in order to find the corresponding network UUID which should be stored
in the newly created NIC object.

Params passed in _ApplyNicMods() include the value passed by user
for the network. Thus a lookup must be done first in order to find
the corresponding network UUID. Then compare it with the existing
value and act accordingly.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoFix another docstring typo
Iustin Pop [Thu, 21 Feb 2013 14:10:56 +0000 (15:10 +0100)]
Fix another docstring typo

… no comment :)

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

7 years agoLocking fixes regarding Issue 324
Dimitris Aragiorgis [Tue, 19 Feb 2013 17:41:48 +0000 (19:41 +0200)]
Locking fixes regarding Issue 324

LUNetworkConnect/LUNetworkDisconnect, in case locking is used, might
lock instances that exist in the requested node group. The acquired
locks should be checked if they are correct at the beginning of
CheckPrereq() via _CheckNodeGroupInstances().

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

7 years agoFix small typo in a docstring
Iustin Pop [Wed, 20 Feb 2013 14:57:10 +0000 (15:57 +0100)]
Fix small typo in a docstring

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

7 years agocheck-news: Ensure release date is within three days
Michael Hanselmann [Wed, 20 Feb 2013 12:58:04 +0000 (13:58 +0100)]
check-news: Ensure release date is within three days

Commit b6ad806f contained a date a month in the future. With this patch
all release dates in NEWS may at most be three days into the future
(assuming the build machine's clock is correct).

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

7 years agoNEWS: Fix the fix in commit 82b6f9a
Michael Hanselmann [Wed, 20 Feb 2013 12:41:59 +0000 (13:41 +0100)]
NEWS: Fix the fix in commit 82b6f9a

February 2, 2013 was a Wednesday, not a Thursday.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoNEWS: Fix release date for 2.7.0 beta1
Michael Hanselmann [Wed, 20 Feb 2013 11:43:47 +0000 (12:43 +0100)]
NEWS: Fix release date for 2.7.0 beta1

Commit b6ad806f used a wrong date.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoMinor fixes regarding nic.network change
Dimitris Aragiorgis [Tue, 19 Feb 2013 16:28:42 +0000 (18:28 +0200)]
Minor fixes regarding nic.network change

Make LookupNetwork() return None in case target is None. This fixes
Issue 380. Rapi passes network=None and the lookup should not fail.

Make network client aware of new nic.network.gnt-network info shows
the IPs of each instance inside the network.  It parses nic.networks
field of QueryInstances() output, which now is a list of uuids an
not names.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoFix issue 378
Dimitris Aragiorgis [Tue, 19 Feb 2013 14:45:30 +0000 (16:45 +0200)]
Fix issue 378

In case a NIC is not inside a network then netinfo None. Thus
netinfo["name"] fails.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoNEWS: Mention multi-alloc and fix typo
Michael Hanselmann [Thu, 14 Feb 2013 15:55:07 +0000 (16:55 +0100)]
NEWS: Mention multi-alloc and fix typo

As far as I could see, the ability to allocate multiple instances via
RAPI was not mentioned at all. A superfluous word is also removed.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

7 years agoAdd cfgupgrade for changing nic.network to uuid
Dimitris Aragiorgis [Sat, 16 Feb 2013 13:19:40 +0000 (15:19 +0200)]
Add cfgupgrade for changing nic.network to uuid

This gets all network info from config_data and parses
all instances and their NICs and makes the substitution.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoRemove useless code in backend for network hooks
Dimitris Aragiorgis [Sat, 16 Feb 2013 13:19:39 +0000 (15:19 +0200)]
Remove useless code in backend for network hooks

In backend NICs arrive with netinfo filled. If nic.network is not None
nic.netinfo is not too. Thus all the info is derived from HooksDict()
and nic.network must not be checked.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoShow network name and not uuid in instance info
Dimitris Aragiorgis [Sat, 16 Feb 2013 13:19:38 +0000 (15:19 +0200)]
Show network name and not uuid in instance info

This was before the case too. Now is bit trickier because nic.network
is uuid. Info must derive from nic.netinfo.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoAdd IQ_NETWORKS in query tests
Dimitris Aragiorgis [Sat, 16 Feb 2013 13:19:37 +0000 (15:19 +0200)]
Add IQ_NETWORKS in query tests

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoImplement network locking in Instance queries
Dimitris Aragiorgis [Sat, 16 Feb 2013 13:19:36 +0000 (15:19 +0200)]
Implement network locking in Instance queries

This is needed in case more info than each nic's network uuid is to
be returned. We need to lock networks to get valid data. For now
only the name is returned as an extra field. All other can be added
with trivial effort.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoChanges in query to support nic.network as uuid
Dimitris Aragiorgis [Sat, 16 Feb 2013 13:19:35 +0000 (15:19 +0200)]
Changes in query to support nic.network as uuid

Queries now return the network uuid as well as it's name. Here we
only use info provided be LUInstanceQueryData context.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoModify query LUs to supoprt nic.network as uuid
Dimitris Aragiorgis [Sat, 16 Feb 2013 13:19:34 +0000 (15:19 +0200)]
Modify query LUs to supoprt nic.network as uuid

Make _InstanceQuery gather all network info related to instance's
NICs and in case of NETQ_INST in _NetworkQuery get all network
uuids directly from nic.network

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoAdd GetInstanceNetworks() config method
Dimitris Aragiorgis [Sat, 16 Feb 2013 13:19:33 +0000 (15:19 +0200)]
Add GetInstanceNetworks() config method

This will be needed for Instance Queries. It walks through the
instance's NICs and returns a list network uuids that the NICs
are attached to.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agocmdlib changes to support nic.network as uuid
Dimitris Aragiorgis [Sat, 16 Feb 2013 13:19:32 +0000 (15:19 +0200)]
cmdlib changes to support nic.network as uuid

Refactor Instance related LUs to support nic.network as
a uuid. This removes all the unnecessary invocations to
LookupNetwork().

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Helga Velroyen <helgav@google.com>

7 years agoMake network config methods take uuid as argument
Dimitris Aragiorgis [Sat, 16 Feb 2013 13:19:31 +0000 (15:19 +0200)]
Make network config methods take uuid as argument

This will be needed in the following patches where nic.network
will refer to network's uuid and not name.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Helga Velroyen <helgav@google.com>