[Libguestfs] [PATCH 1/4] generator: add always-available optgroups

[Richard W.M. Jones]

On Wed, May 21, 2014 at 10:47:04AM +0200, Pino Toscano wrote:
> On Tuesday 20 May 2014 21:40:21 Richard W.M. Jones wrote:
> > On Tue, May 20, 2014 at 07:54:45PM +0200, Pino Toscano wrote:
> > > Support the possibility to have optional groups always enabled (e.g.
> > > because they were present in the past, and they need to be kept for
> > > users).
> > > Add and use few helper optgroups-related functions to deal also with
> > > them.
> > 
> > What do you think about the attached addition to this patch?
> 
>  let optgroups_names_all =
> -  List.sort compare (optgroups_names @ optgroups_available)
> +  List.sort compare (optgroups_names @ optgroups_retired)
> 
> Wouldn't this give duplicate values if a feature appears in both 
> internal_optgroups_available/optgroups_retired and the auto-generated 
> optgroups_names?

If we only put retired optgroups into the optgroups_retired list then
it's not a problem.

Actually your comment does explain what the loop does that I removed
in my patch.  I was wondering what it was for since it would have no
effect in the current code (where optgroups_retired are only retired
optgroups).

I think a better way to do this is to have the generator check -- and
die with an error -- if retired optgroups appear in the list of actions.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org