On Thu, Feb 13, 2014 at 3:46 PM, Dimitris Aragiorgis <[email protected]> wrote:
> During backup import/export SafeConfigParser() is used to
> save/restore instance's configuration. There is a possibility if an
> export is done with a different Ganeti version, a specific value not
> to be saved during export (e.g. the NIC/Disk name) but still
> requested during import.
>
> With this patch we override the get() method of SafeConfigParser()
> and catch NoOptionError if raised and return None. Additionally we
> translate "None" values read from .ini file into None.
>
> Signed-off-by: Dimitris Aragiorgis <[email protected]>
> ---
>  lib/objects.py |   15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/lib/objects.py b/lib/objects.py
> index 358f108..6aca6fc 100644
> --- a/lib/objects.py
> +++ b/lib/objects.py
> @@ -2159,7 +2159,8 @@ class Network(TaggableObject):
>      return obj
>
>
> -class SerializableConfigParser(ConfigParser.SafeConfigParser):
> +# need to inherit object in order to use super()
> +class SerializableConfigParser(ConfigParser.SafeConfigParser, object):
>    """Simple wrapper over ConfigParse that allows serialization.
>
>    This class is basically ConfigParser.SafeConfigParser with two
> @@ -2181,6 +2182,18 @@ class 
> SerializableConfigParser(ConfigParser.SafeConfigParser):
>      cfp.readfp(buf)
>      return cfp
>
> +  def get(self, section, option, **kwargs):
> +    value = None
> +    try:
> +      value = super(SerializableConfigParser, self).get(section, option,
> +                                                        **kwargs)
> +      if value.lower() == constants.VALUE_NONE:
> +        value = None
> +    except ConfigParser.NoOptionError:
> +      pass
> +
> +    return value
> +
>
>  class LvmPvInfo(ConfigObject):
>    """Information about an LVM physical volume (PV).
> --
> 1.7.10.4
>

I think we should at least log that an error happened. Silently
accepting it seems to me extremely dangerous.

Also, I'm not confident enough about this part of the code to say that
all the values can be optional if not found. So, what about
specifically checking whether we have been looking for the disk name
and nick name and ONLY in that case returning None instead of the
exception?
This way, we know that we are modifying only the behavior related to
the new configuration fields.

Cheers,
Michele

-- 
Google Germany GmbH
Dienerstr. 12
80331 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores

Reply via email to