LGTM, thanks.

Since the previous patch breaks the Haskell compability, I will push this
patch with the patch that fixes it.

Cheers,
Helga


On Tue, Jan 29, 2013 at 9:40 AM, Dimitris Aragiorgis <[email protected]>wrote:

> Make _UnlockedLookupNetwork() raise OpPrereqError (instead of returning
> None) in case it does not find the requested network. Remove useless and
> duplicate code such as:
>
> if net_uuid is None:
>   raise...
>
> Signed-off-by: Dimitris Aragiorgis <[email protected]>
> ---
>  lib/cmdlib.py |   57
> ++++++++++++++++++---------------------------------------
>  lib/config.py |   30 +++++++++---------------------
>  2 files changed, 27 insertions(+), 60 deletions(-)
>
> diff --git a/lib/cmdlib.py b/lib/cmdlib.py
> index 5798c6f..6556f70 100644
> --- a/lib/cmdlib.py
> +++ b/lib/cmdlib.py
> @@ -1551,20 +1551,16 @@ def _NICToTuple(lu, nic):
>    @param nic: nic to convert to hooks tuple
>
>    """
> -  ip = nic.ip
> -  mac = nic.mac
>    cluster = lu.cfg.GetClusterInfo()
>    filled_params = cluster.SimpleFillNIC(nic.nicparams)
>    mode = filled_params[constants.NIC_MODE]
>    link = filled_params[constants.NIC_LINK]
> -  net = nic.network
>    netinfo = None
> -  if net:
> -    net_uuid = lu.cfg.LookupNetwork(net)
> -    if net_uuid:
> -      nobj = lu.cfg.GetNetwork(net_uuid)
> -      netinfo = objects.Network.ToDict(nobj)
> -  return (ip, mac, mode, link, net, netinfo)
> +  if nic.network:
> +    net_uuid = lu.cfg.LookupNetwork(nic.network)
> +    netinfo = objects.Network.ToDict(lu.cfg.GetNetwork(net_uuid))
> +
> +  return (nic.ip, nic.mac, mode, link, nic.network, netinfo)
>
>
>  def _NICListToTuple(lu, nics):
> @@ -13460,12 +13456,12 @@ class LUInstanceSetParams(LogicalUnit):
>      elif new_net != old_net:
>
>        def get_net_prefix(net):
> +        mac_prefix = None
>          if net:
>            uuid = self.cfg.LookupNetwork(net)
> -          if uuid:
> -            nobj = self.cfg.GetNetwork(uuid)
> -            return nobj.mac_prefix
> -        return None
> +          mac_prefix = self.cfg.GetNetwork(uuid).mac_prefix
> +
> +        return mac_prefix
>
>        new_prefix = get_net_prefix(new_net)
>        old_prefix = get_net_prefix(old_net)
> @@ -16215,11 +16211,15 @@ class LUNetworkAdd(LogicalUnit):
>        raise errors.OpPrereqError("Network must be given",
>                                   errors.ECODE_INVAL)
>
> -    uuid = self.cfg.LookupNetwork(self.op.network_name)
> -
> -    if uuid:
> -      raise errors.OpPrereqError(("Network with name '%s' already exists"
> %
> -                                  self.op.network_name),
> errors.ECODE_EXISTS)
> +    try:
> +      existing_uuid = self.cfg.LookupNetwork(self.op.network_name)
> +    except errors.OpPrereqError:
> +      pass
> +    else:
> +      raise errors.OpPrereqError("Desired network name '%s' already
> exists as a"
> +                                 " network (UUID: %s)" %
> +                                 (self.op.network_name, existing_uuid),
> +                                 errors.ECODE_EXISTS)
>
>      # Check tag validity
>      for tag in self.op.tags:
> @@ -16307,10 +16307,6 @@ class LUNetworkRemove(LogicalUnit):
>    def ExpandNames(self):
>      self.network_uuid = self.cfg.LookupNetwork(self.op.network_name)
>
> -    if not self.network_uuid:
> -      raise errors.OpPrereqError(("Network '%s' not found" %
> -                                  self.op.network_name),
> errors.ECODE_NOENT)
> -
>      self.share_locks[locking.LEVEL_NODEGROUP] = 1
>      self.needed_locks = {
>        locking.LEVEL_NETWORK: [self.network_uuid],
> @@ -16379,9 +16375,6 @@ class LUNetworkSetParams(LogicalUnit):
>
>    def ExpandNames(self):
>      self.network_uuid = self.cfg.LookupNetwork(self.op.network_name)
> -    if self.network_uuid is None:
> -      raise errors.OpPrereqError(("Network '%s' not found" %
> -                                  self.op.network_name),
> errors.ECODE_NOENT)
>
>      self.needed_locks = {
>        locking.LEVEL_NETWORK: [self.network_uuid],
> @@ -16654,14 +16647,7 @@ class LUNetworkConnect(LogicalUnit):
>      self.network_link = self.op.network_link
>
>      self.network_uuid = self.cfg.LookupNetwork(self.network_name)
> -    if self.network_uuid is None:
> -      raise errors.OpPrereqError("Network '%s' does not exist" %
> -                                 self.network_name, errors.ECODE_NOENT)
> -
>      self.group_uuid = self.cfg.LookupNodeGroup(self.group_name)
> -    if self.group_uuid is None:
> -      raise errors.OpPrereqError("Group '%s' does not exist" %
> -                                 self.group_name, errors.ECODE_NOENT)
>
>      self.needed_locks = {
>        locking.LEVEL_INSTANCE: [],
> @@ -16790,14 +16776,7 @@ class LUNetworkDisconnect(LogicalUnit):
>      self.group_name = self.op.group_name
>
>      self.network_uuid = self.cfg.LookupNetwork(self.network_name)
> -    if self.network_uuid is None:
> -      raise errors.OpPrereqError("Network '%s' does not exist" %
> -                                 self.network_name, errors.ECODE_NOENT)
> -
>      self.group_uuid = self.cfg.LookupNodeGroup(self.group_name)
> -    if self.group_uuid is None:
> -      raise errors.OpPrereqError("Group '%s' does not exist" %
> -                                 self.group_name, errors.ECODE_NOENT)
>
>      self.needed_locks = {
>        locking.LEVEL_INSTANCE: [],
> diff --git a/lib/config.py b/lib/config.py
> index 9d87c2b..55bef8d 100644
> --- a/lib/config.py
> +++ b/lib/config.py
> @@ -276,10 +276,9 @@ class ConfigWriter:
>      prefix = None
>      if net:
>        net_uuid = self._UnlockedLookupNetwork(net)
> -      if net_uuid:
> -        nobj = self._UnlockedGetNetwork(net_uuid)
> -        if nobj.mac_prefix:
> -          prefix = nobj.mac_prefix
> +      nobj = self._UnlockedGetNetwork(net_uuid)
> +      if nobj.mac_prefix:
> +        prefix = nobj.mac_prefix
>
>      return prefix
>
> @@ -365,8 +364,7 @@ class ConfigWriter:
>
>      """
>      net_uuid = self._UnlockedLookupNetwork(net)
> -    if net_uuid:
> -      self._UnlockedReleaseIp(net_uuid, address, ec_id)
> +    self._UnlockedReleaseIp(net_uuid, address, ec_id)
>
>    @locking.ssynchronized(_config_lock, shared=1)
>    def GenerateIp(self, net, ec_id):
> @@ -410,8 +408,7 @@ class ConfigWriter:
>
>      """
>      net_uuid = self._UnlockedLookupNetwork(net)
> -    if net_uuid:
> -      return self._UnlockedReserveIp(net_uuid, address, ec_id)
> +    return self._UnlockedReserveIp(net_uuid, address, ec_id)
>
>    @locking.ssynchronized(_config_lock, shared=1)
>    def ReserveLV(self, lv_name, ec_id):
> @@ -1452,9 +1449,8 @@ class ConfigWriter:
>      for nic in instance.nics:
>        if nic.network is not None and nic.ip is not None:
>          net_uuid = self._UnlockedLookupNetwork(nic.network)
> -        if net_uuid:
> -          # Return all IP addresses to the respective address pools
> -          self._UnlockedCommitIp(constants.RELEASE_ACTION, net_uuid,
> nic.ip)
> +        # Return all IP addresses to the respective address pools
> +        self._UnlockedCommitIp(constants.RELEASE_ACTION, net_uuid, nic.ip)
>
>      del self._config_data.instances[instance_name]
>      self._config_data.cluster.serial_no += 1
> @@ -2469,12 +2465,6 @@ class ConfigWriter:
>      if check_uuid:
>        self._EnsureUUID(net, ec_id)
>
> -    existing_uuid = self._UnlockedLookupNetwork(net.name)
> -    if existing_uuid:
> -      raise errors.OpPrereqError("Desired network name '%s' already"
> -                                 " exists as a network (UUID: %s)" %
> -                                 (net.name, existing_uuid),
> -                                 errors.ECODE_EXISTS)
>      net.serial_no = 1
>      self._config_data.networks[net.uuid] = net
>      self._config_data.cluster.serial_no += 1
> @@ -2494,7 +2484,8 @@ class ConfigWriter:
>      for net in self._config_data.networks.values():
>        if net.name == target:
>          return net.uuid
> -    return None
> +    raise errors.OpPrereqError("Network '%s' not found" % target,
> +                               errors.ECODE_NOENT)
>
>    @locking.ssynchronized(_config_lock, shared=1)
>    def LookupNetwork(self, target):
> @@ -2541,9 +2532,6 @@ class ConfigWriter:
>
>      """
>      net_uuid = self._UnlockedLookupNetwork(net)
> -    if net_uuid is None:
> -      return None
> -
>      node_info = self._UnlockedGetNodeInfo(node)
>      nodegroup_info = self._UnlockedGetNodeGroup(node_info.group)
>      netparams = nodegroup_info.networks.get(net_uuid, None)
> --
> 1.7.10.4
>
> --
> You received this message because you are subscribed to the Google Groups
> "ganeti-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> For more options, visit https://groups.google.com/groups/opt_out.
>
>
>

Reply via email to