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@xxxxxxxxxx>
> ---
> 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 :|
_______________________________________________
Libosinfo mailing list
Libosinfo@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libosinfo
