[virt-tools-list] [libosinfo PATCHv3 10/12] Add osinfo_install_config_new_for_script

[Christophe Fergeau]

On Thu, Dec 13, 2012 at 02:56:09AM +0200, Zeeshan Ali (Khattak) wrote:
> On Wed, Dec 12, 2012 at 5:18 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> > In order to be able to automatically transform configuration parameters set
> > on OsinfoInstallConfig instances (ie translate from generic value to
> > OS-specific value),
> 
> Hmm.. this seemingly small change is rather a big change in terms of
> API design as so far we have kept install config separate from install
> script. They only meet each other when generating scripts from
> template.
> 
> Since the OS-specific values are actually needed when transforming the
> template to script (i-e in osinfo_install_script_generate_config_xml),
>  I wonder if there is another less intrusive (in terms of API) way to
> achieve the goal.

I was initially considering plugging into
osinfo_install_script_generate_config_xml or some such function, but then
it made more sense to me to do things the way it's done in this patch
series.
The install script XML contains a <config> section describing the valid
parameters for the install script. Codewise, we have a OsinfoInstallScript
which can contain a list of OsinfoInstallConfigParam. These objects are the
result of parsing the install script XML.

We also have an OsinfoInstallConfig class, which, as you mention, is
independent of the OsinfoInstallScript and OsinfoInstallConfigParam classes
Note the naming which could be confusing, OsinfoInstallConfig and
OsinfoInstallConfigParam have nothing to do with each other at the moment.

This separation between OsinfoInstallConfigParam and OsinfoInstallConfig
means that one has to refer to OsinfoInstallScript to know which parameters
are valid/required when filling an OsinfoInstallConfig object with data.

Regardless of the datamap stuff, it makes sense to me to associate the
OsinfoInstallConfig 'schema' (that is a list of
OsinfoInstallConfigParam) with the OsinfoInstallConfig instance that is
being filled. This could also allow us in the future to reject setting
'useless' arguments, to do argument validation (this arg only has X, Y
and Z as valid values, this int must be between 1 and 10, ...).

I can look at not associating OsinfoInstallConfigParam with
OsinfoInstallConfig, but this will require a bit of refactoring,
and I think this alternative approach would look more hackish than what
this series is doing.

In short, I consider the subseries:

Add osinfo_install_config_new_for_script
Add OsinfoInstallConfig::valid-params property
OsinfoInstallScript: Use an OsinfoInstallConfigParamList
Set id in osinfo_install_config_param_new
Add OsinfoInstallConfigParamList

as a nice improvement to the codebase even if there were not the datamap
changes on top of it.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20121213/c12ae399/attachment.sig>