On Thu, Dec 01, 2016 at 12:34:11PM +0200, Yiannis Tsiouris wrote:
> This patch extends 'gnt-instance modify' by allowing a user to remove a
> list of public/private OS parameters from an instance. This can be
> useful before performing a reinstall to a new OS provider. Example
> usage:
>
> $ gnt-instance modify --remove-os-parameters parm1,parm2 <instance_name>
>
> or
>
> $ gnt-instance modify --remove-os-parameters-private parm3 <instance_name>
>
> Signed-off-by: Yiannis Tsiouris <[email protected]>
Mostly LGTM, see a couple of inline comments.
Also, a thought: I wonder could we merge the --clear-os-parameters functionality
into --remove-os-parameters by having a special '*' parameter. If it's the only
one present in the list, it means remove everything. It would mean we could get
rid of all the boilerplate associated with parsing (and serialising) 2 extra
options.
Thanks,
Brian.
> + for osp in remove_osparams:
> + if osp in public_parms:
> + raise errors.OpPrereqError("Requested both removal and addition of
> "
> + "param %s" % osp)
> +
> + if osp in self.instance.osparams:
> + self.os_inst_removed.append(osp)
> + del self.instance.osparams[osp]
Maybe add an else: print warning about removing an option that's not set.
> +
> + for osp in remove_osparams_private:
> + if osp in private_parms:
> + raise errors.OpPrereqError("Requested both removal and addition of
> "
> + "param %s" % osp)
> +
> + if osp in self.instance.osparams_private:
> + self.os_inst_private_removed.append(osp)
> + del self.instance.osparams_private[osp]
Warn here too.
