[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Wed, Jul 25, 2012 at 3:18 PM, Daniel P. Berrange <berrange 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
>
>> +                           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
>
>> +                           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;

Thinking a bit about it.
In my implementation I'm keeping 2 lists of strings. One of required
configs and another one of optional configs,
So, I'm not keeping the policy on my lists, I'm keeping the config
property name. With this in mind, how this enum could help, so?
If you are thinking about a more efficient way to keep the lists,
please, let me know.


>
>
>   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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]