[Libosinfo] [libosinfo PATCH v2 1/5] install-script: Add _(get|set)_preferred_injection_method()

Daniel P. Berrangé berrange at redhat.com
Wed Dec 19 11:02:47 UTC 2018


On Mon, Dec 17, 2018 at 08:10:57PM +0100, Fabiano Fidêncio wrote:
> Those new methods are going to be used to tell the install-scripts
> whether the injection-method that's going to be used is. We have to do
> so as the command-line may be different depending on the
> injection-method used, for example:
> - fedora using cdrom, disk or floppy: ks=hd:/(vda|sda)/fedora.ks
> - fedora using initrd: ks=file:/fedora.ks
> 
> It's important to mention that although the methods are taking GFlags,
> those are treated as GEnum and only one value is expected to be set, as
> mentioned in the documentation.
> 
> Also, mind that the usage of osinfo_entity_set_param() to store the
> nick of the OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_* is intentional as
> the nick is exactly what's going to be used in the install-scripts to
> generate the proper command-line.
> 
> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
> ---
>  osinfo/libosinfo.syms          |  3 ++
>  osinfo/osinfo_install_script.c | 95 ++++++++++++++++++++++++++++++++++
>  osinfo/osinfo_install_script.h |  4 ++
>  3 files changed, 102 insertions(+)
> 
> diff --git a/osinfo/libosinfo.syms b/osinfo/libosinfo.syms
> index 45a7219..0272e94 100644
> --- a/osinfo/libosinfo.syms
> +++ b/osinfo/libosinfo.syms
> @@ -543,6 +543,9 @@ LIBOSINFO_1.3.0 {
>  	osinfo_imagelist_get_type;
>  	osinfo_imagelist_new;
>  
> +	osinfo_install_script_get_preferred_injection_method;
> +	osinfo_install_script_set_preferred_injection_method;
> +
>  	osinfo_media_supports_installer_script;
>  
>  	osinfo_os_add_image;
> diff --git a/osinfo/osinfo_install_script.c b/osinfo/osinfo_install_script.c
> index 016f850..68f0a9d 100644
> --- a/osinfo/osinfo_install_script.c
> +++ b/osinfo/osinfo_install_script.c
> @@ -64,6 +64,7 @@ enum {
>      PROP_PRODUCT_KEY_FORMAT,
>      PROP_PATH_FORMAT,
>      PROP_AVATAR_FORMAT,
> +    PROP_PREFERRED_INJECTION_METHOD
>  };
>  
>  typedef struct _OsinfoInstallScriptGenerateData OsinfoInstallScriptGenerateData;
> @@ -105,6 +106,11 @@ osinfo_install_script_set_property(GObject    *object,
>                                      data);
>          break;
>  
> +    case PROP_PREFERRED_INJECTION_METHOD:
> +        osinfo_install_script_set_preferred_injection_method(script,
> +                                                             g_value_get_flags(value));
> +        break;
> +
>      default:
>          /* We don't have any other property... */
>          G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
> @@ -151,6 +157,11 @@ osinfo_install_script_get_property(GObject    *object,
>                             osinfo_install_script_get_avatar_format(script));
>          break;
>  
> +    case PROP_PREFERRED_INJECTION_METHOD:
> +        g_value_set_flags(value,
> +                          osinfo_install_script_get_preferred_injection_method(script));
> +        break;
> +
>      default:
>          /* We don't have any other property... */
>          G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
> @@ -254,6 +265,17 @@ osinfo_install_script_class_init(OsinfoInstallScriptClass *klass)
>                                      PROP_AVATAR_FORMAT,
>                                      pspec);
>  
> +    pspec = g_param_spec_flags("preferred-injection-method",
> +                               "Preferred Injection Method",
> +                               _("The preferred injection method"),
> +                               OSINFO_TYPE_INSTALL_SCRIPT_INJECTION_METHOD,
> +                               OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_DISK, /* default value */
> +                               G_PARAM_READABLE |

You've provided a setter, so why only G_PARAM_READABLE, and not WRITABLE too ?

> +                               G_PARAM_STATIC_STRINGS);
> +    g_object_class_install_property(g_klass,
> +                                    PROP_PREFERRED_INJECTION_METHOD,
> +                                    pspec);
> +
>      g_type_class_add_private(klass, sizeof(OsinfoInstallScriptPrivate));
>  }
>  
> @@ -1765,6 +1787,79 @@ gboolean osinfo_install_script_get_needs_internet(OsinfoInstallScript *script)
>           FALSE);
>  }
>  
> +/**
> + * osinfo_install_script_set_preferred_injection_method:
> + * @script: the install script
> + * @method: one of the injection methods:
> + * OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_CDROM,
> + * OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_DISK,
> + * OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_FLOPPY,
> + * OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_INITRD,
> + * OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_WEB
> + *
> + * Set the preferred injection method to be used with the @script
> + */
> +void osinfo_install_script_set_preferred_injection_method(OsinfoInstallScript *script,
> +                                                          OsinfoInstallScriptInjectionMethod method)
> +{
> +    GFlagsClass *flags_class;
> +    guint supported_methods;
> +    guint i;
> +
> +    supported_methods = osinfo_install_script_get_injection_methods(script);
> +    if ((method & supported_methods) == 0) {
> +        g_warning("The injection-method passed is not supported by the install-script");
> +        return;

g_warning is not appropriate for something which is likely to happen
depending on data. We should have this return a gboolean error status.
A caller can use g_warning if it desires.

> +    }
> +

We should do a check here to see if multiple bits are set in method
before trying to set the parameter and again return an error status

> +    flags_class = g_type_class_ref(OSINFO_TYPE_INSTALL_SCRIPT_INJECTION_METHOD);
> +    for (i = 0; i < flags_class->n_values; i++) {
> +        if ((flags_class->values[i].value & method) != 0) {
> +            osinfo_entity_set_param(OSINFO_ENTITY(script),
> +                                    OSINFO_INSTALL_SCRIPT_PROP_PREFERRED_INJECTION_METHOD,
> +                                    flags_class->values[i].value_nick);
> +            break;
> +        }
> +    }
> +    g_type_class_unref(flags_class);
> +}
> +
> +/**
> + * osinfo_install_script_get_preferred_injection_method:
> + * @script: the install script
> + *
> + * Returns: the preferred injection method for the script. If none is set and
> + * OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_DISK is supported,
> + * OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_DISK is returned, otherwise
> + * OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_INITRD is returned.
> + */
> +OsinfoInstallScriptInjectionMethod
> +osinfo_install_script_get_preferred_injection_method(OsinfoInstallScript *script)
> +{
> +    GFlagsClass *flags_class;
> +    GFlagsValue *value;
> +    const gchar *nick;
> +    guint supported_methods;
> +
> +    nick = osinfo_entity_get_param_value(OSINFO_ENTITY(script),
> +            OSINFO_INSTALL_SCRIPT_PROP_PREFERRED_INJECTION_METHOD);
> +
> +    if (nick == NULL) {
> +        supported_methods = osinfo_install_script_get_injection_methods(script);
> +        if ((supported_methods & OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_DISK) != 0)
> +            return OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_DISK;
> +        else if ((supported_methods & OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_INITRD) != 0)
> +            return OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_INITRD;
> +        else
> +            g_return_val_if_reached(OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_DISK);

g_return_val_if_reached is pretty pointless. A simple 'return' is sufficient IMHO

> +    }
> +
> +    flags_class = g_type_class_ref(OSINFO_TYPE_INSTALL_SCRIPT_INJECTION_METHOD);
> +    value = g_flags_get_value_by_nick(flags_class, nick);
> +    g_type_class_unref(flags_class);
> +
> +    return value->value;
> +}

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|




More information about the Libosinfo mailing list