On Fri, Dec 02, 2016 at 04:53:51PM +0200, Yiannis Tsiouris wrote: > On 12/02, Brian Foley wrote: > > 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. > > I agree with everything. > > > 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. > > Hmm, that's an interesting point. I think this could be more clear both for > the > user and the code. I could do that! Just to be clear: I should treat the > string > '*' as a "special" OS parameter; this doesn't mean that I should try to globe > based on '*' i.e. 'param*' doesn't expand to 'param1', 'param2', etc., right?
Good point. Just having literal "*" as a special parameter is the simplest, but I can imagine globbing, or perhaps even full regexp matching might well be useful too. We can't just use Python's glob.glob because that operates on directories only, but we could make a variant of utils.text.DnsNameGlobPattern to match with regexps for us. Cheers, Brian.
