[virt-tools-list] [PATCH 4/6] Add functions to know if a config can/must be set

Fabiano Fidêncio fabiano at fidencio.org
Thu Jul 26 10:27:35 UTC 2012


On Wed, Jul 25, 2012 at 3:18 PM, Daniel P. Berrange <berrange at redhat.com> wrote:
> On Tue, Jul 24, 2012 at 10:05:16PM +0200, Fabiano Fidêncio wrote:
>> Now we need to set what are the configs that will be used in each
>> script. To set it, just add, in the .xml's script file:
>> <config>
>>   <param name="..." policy="mandatory"|"optional"/>
>> </config>
>>
>> With these configs loaded on our db, applications that uses/will use
>> libosinfo can check if some config *can* be set using:
>> osinfo_install_script_is_config(OsinfoInstallScript *script,
>>                                 const gchar* config);
>>
>> And if some config *must* be set using:
>> osinfo_install_script_is_config_required(OsinfoInstallScript *script,
>>                                          const gchar* config);
>> ---
>>  data/install-scripts/fedora.xml           |    6 ++++
>>  data/install-scripts/windows-sif.xml      |    5 +++
>>  data/install-scripts/windows-unattend.xml |    9 ++++++
>>  data/schemas/libosinfo.rng                |   12 ++++++++
>>  osinfo/libosinfo.syms                     |    2 ++
>>  osinfo/osinfo_install_script.c            |   47 +++++++++++++++++++++++++++++
>>  osinfo/osinfo_install_script.h            |    9 +++++-
>>  osinfo/osinfo_loader.c                    |   19 ++++++++++++
>>  8 files changed, 108 insertions(+), 1 deletion(-)
>>
>> diff --git a/data/install-scripts/fedora.xml b/data/install-scripts/fedora.xml
>> index 338a570..a608cdb 100644
>> --- a/data/install-scripts/fedora.xml
>> +++ b/data/install-scripts/fedora.xml
>> @@ -1,6 +1,12 @@
>>  <libosinfo version="0.0.1">
>>    <install-script id='http://fedoraproject.org/scripts/fedora/jeos'>
>>      <profile>jeos</profile>
>> +    <config>
>> +      <param name="admin-password" policy="optional"/>
>> +      <param name="l10n-keyboard" policy="optional"/>
>> +      <param name="l10n-language" policy="optional"/>
>> +      <param name="l10n-timezone" policy="optional"/>
>> +    </config>
>>      <template filename="fedora.ks">
>>        <xsl:stylesheet
>>          xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
>> diff --git a/data/install-scripts/windows-sif.xml b/data/install-scripts/windows-sif.xml
>> index 46b24ae..77fc2c5 100644
>> --- a/data/install-scripts/windows-sif.xml
>> +++ b/data/install-scripts/windows-sif.xml
>> @@ -1,6 +1,11 @@
>>  <libosinfo version="0.0.1">
>>    <install-script id='http://microsoft.com/windows/sif'>
>>      <profile>jeos</profile>
>> +    <config>
>> +      <param name="admin-password" policy="optional"/>
>> +      <param name="reg-product-key" policy="required"/>
>> +      <param name="user-realname" policy="required"/>
>> +    </config>
>>      <template filename="windows.sif">
>>        <xsl:stylesheet
>>          xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
>> diff --git a/data/install-scripts/windows-unattend.xml b/data/install-scripts/windows-unattend.xml
>> index 20e3e23..5698bc8 100644
>> --- a/data/install-scripts/windows-unattend.xml
>> +++ b/data/install-scripts/windows-unattend.xml
>> @@ -1,6 +1,15 @@
>>  <libosinfo version="0.0.1">
>>    <install-script id='http://microsoft.com/windows/unattend'>
>>      <profile>jeos</profile>
>> +    <config>
>> +      <param name="admin-password" policy="optional"/>
>> +      <param name="hardware-arch" policy="optional"/>
>> +      <param name="l10n-language" policy="optional"/>
>> +      <param name="user-login" policy="optional"/>
>> +      <param name="user-password" policy="optional"/>
>> +      <param name="user-realname" policy="optional"/>
>> +      <param name="reg-product-key" policy="required"/>
>> +    </config>
>>      <template filename="windows.xml">
>>        <xsl:stylesheet
>>          xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
>> diff --git a/data/schemas/libosinfo.rng b/data/schemas/libosinfo.rng
>> index 7c8d7f7..1392f74 100644
>> --- a/data/schemas/libosinfo.rng
>> +++ b/data/schemas/libosinfo.rng
>> @@ -410,6 +410,12 @@
>>        <element name='profile'>
>>          <text/>
>>        </element>
>> +      <element name='config'>
>> +        <attribute name="name"/>
>> +        <attribute name="policy">
>> +          <ref name='policies'/>
>> +        </attribute>
>> +      </element>
>>        <element name='product-key-format'>
>>          <text/>
>>        </element>
>> @@ -479,4 +485,10 @@
>>        <param name="pattern">\w+://.*</param>
>>      </data>
>>    </define>
>> +
>> +  <define name='policies'>
>> +    <data type="string">
>> +      <param name="pattern">required|optional</param>
>> +    </data>
>> +  </define>
>>  </grammar>
>> diff --git a/osinfo/libosinfo.syms b/osinfo/libosinfo.syms
>> index 5a7de7b..2d64ebb 100644
>> --- a/osinfo/libosinfo.syms
>> +++ b/osinfo/libosinfo.syms
>> @@ -276,6 +276,8 @@ LIBOSINFO_0.2.0 {
>>       osinfo_install_script_generate_output_async;
>>       osinfo_install_script_get_profile;
>>       osinfo_install_script_get_uri;
>> +     osinfo_install_script_is_config;
>> +     osinfo_install_script_is_config_required;
>>       osinfo_install_scriptlist_new;
>>       osinfo_install_scriptlist_new_filtered;
>>       osinfo_install_scriptlist_new_union;
>> diff --git a/osinfo/osinfo_install_script.c b/osinfo/osinfo_install_script.c
>> index b2d7351..6ce1c55 100644
>> --- a/osinfo/osinfo_install_script.c
>> +++ b/osinfo/osinfo_install_script.c
>> @@ -240,6 +240,26 @@ osinfo_install_script_class_init (OsinfoInstallScriptClass *klass)
>>      g_type_class_add_private (klass, sizeof (OsinfoInstallScriptPrivate));
>>  }
>>
>> +static GList *osinfo_install_script_get_config_required(OsinfoInstallScript *script)
>> +{
>> +    return osinfo_entity_get_param_value_list(OSINFO_ENTITY(script),
>> +                                              OSINFO_INSTALL_SCRIPT_PROP_CONFIG_REQUIRED);
>> +}
>> +
>> +static GList *osinfo_install_script_get_config_optional(OsinfoInstallScript *script)
>> +{
>> +    return osinfo_entity_get_param_value_list(OSINFO_ENTITY(script),
>> +                                              OSINFO_INSTALL_SCRIPT_PROP_CONFIG_OPTIONAL);
>> +}
>> +
>> +static GList *osinfo_install_script_get_config(OsinfoInstallScript *script)
>> +{
>> +    GList *required = osinfo_install_script_get_config_required(script);
>> +    GList *optional = osinfo_install_script_get_config_optional(script);
>> +
>> +    return g_list_concat(required, optional);
>> +}
>
> This leaks memory associated with 'optional'
>
>> +
>>  static void
>>  osinfo_install_script_init (OsinfoInstallScript *list)
>>  {
>> @@ -336,6 +356,33 @@ const gchar *osinfo_install_script_get_output_filename(OsinfoInstallScript *scri
>>                                           OSINFO_INSTALL_SCRIPT_PROP_OUTPUT_FILENAME);
>>  }
>>
>> +static gint osinfo_install_script_compare_configs(gconstpointer a, gconstpointer b)
>> +{
>> +    return (g_strcmp0((const gchar *)a, (const gchar *)b));
>> +}
>> +
>> +gboolean osinfo_install_script_is_config(OsinfoInstallScript *script,
>> +                                         const gchar *config)
>> +{
>> +    if (g_list_find_custom(osinfo_install_script_get_config(script),
>
> This leaks the GList you're using


Just to understand, why is this leaking the GList I'm using?

>
>> +                           config,
>> +                           osinfo_install_script_compare_configs) == NULL)
>> +        return FALSE;
>> +
>> +    return TRUE;
>> +}
>> +
>> +gboolean osinfo_install_script_is_config_required(OsinfoInstallScript *script,
>> +                                                  const gchar *config)
>> +{
>> +    if (g_list_find_custom(osinfo_install_script_get_config_required(script),
>
> This leaks the GList you're using

Same here.

>
>> +                           config,
>> +                           osinfo_install_script_compare_configs) == NULL)
>> +        return FALSE;
>> +
>> +    return TRUE;
>> +}
>> +
>>  struct OsinfoInstallScriptGenerate {
>>      GSimpleAsyncResult *res;
>>      OsinfoOs *os;
>> diff --git a/osinfo/osinfo_install_script.h b/osinfo/osinfo_install_script.h
>> index d75ec93..cca257a 100644
>> --- a/osinfo/osinfo_install_script.h
>> +++ b/osinfo/osinfo_install_script.h
>> @@ -50,7 +50,8 @@ typedef struct _OsinfoInstallScriptPrivate OsinfoInstallScriptPrivate;
>>  #define OSINFO_INSTALL_SCRIPT_PROP_PROFILE            "profile"
>>  #define OSINFO_INSTALL_SCRIPT_PROP_PRODUCT_KEY_FORMAT "product-key-format"
>>  #define OSINFO_INSTALL_SCRIPT_PROP_OUTPUT_FILENAME    "output-filename"
>> -
>> +#define OSINFO_INSTALL_SCRIPT_PROP_CONFIG_REQUIRED    "required"
>> +#define OSINFO_INSTALL_SCRIPT_PROP_CONFIG_OPTIONAL    "optional"
>>
>>  /* object */
>>  struct _OsinfoInstallScript
>> @@ -128,6 +129,12 @@ const GFile *osinfo_install_script_generate_output(OsinfoInstallScript *script,
>>                                                     GCancellable *cancellable,
>>                                                     GError **error);
>>
>> +gboolean osinfo_install_script_is_config(OsinfoInstallScript *script,
>> +                                         const gchar *config);
>> +
>> +gboolean osinfo_install_script_is_config_required(OsinfoInstallScript *script,
>> +                                                  const gchar *config);
>
> How about having an enum
>
>    typedef enum {
>      OSINFO_INSTALL_SCRIPT_CONFIG_POLICY_NONE,
>      OSINFO_INSTALL_SCRIPT_CONFIG_POLICY_REQUIRED,
>      OSINFO_INSTALL_SCRIPT_CONFIG_POLICY_OPTIONAL,
>    } OsinfoInstallScriptConfigPolicy;
>
>
>   OsinfoInstallScriptConfigPolicy;
>   osinfo_install_script_get_config_parameter(OsinfoInstallScript *script,
>                                             const gchar *config);
>
> Also I'd probably add an API to list all parameters
>
>   GList *
>   osinfo_install_script_get_config_parameters(OsinfoInstallScript *script);
>
>
>
> Daniel
> --
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|



-- 
Fabiano Fidêncio




More information about the virt-tools-list mailing list