[virt-tools-list] [libosinfo PATCHv2 3/9] Add osinfo_db_identify_media

[Zeeshan Ali (Khattak)]

On Tue, Dec 11, 2012 at 10:17 PM, Christophe Fergeau
<cfergeau at redhat.com> wrote:
> ---
>  osinfo/libosinfo.syms |  1 +
>  osinfo/osinfo_db.c    | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  osinfo/osinfo_db.h    |  2 ++
>  osinfo/osinfo_media.c |  5 +++-
>  test/test-isodetect.c | 10 ++++----
>  tools/osinfo-detect.c | 13 ++++++----
>  6 files changed, 91 insertions(+), 11 deletions(-)
>
> diff --git a/osinfo/libosinfo.syms b/osinfo/libosinfo.syms
> index c5ab005..8d1e27a 100644
> --- a/osinfo/libosinfo.syms
> +++ b/osinfo/libosinfo.syms
> @@ -386,6 +386,7 @@ LIBOSINFO_0.2.3 {
>         osinfo_db_add_datamap;
>         osinfo_db_get_datamap;
>         osinfo_db_get_datamap_list;
> +       osinfo_db_identify_media;
>
>         osinfo_install_config_new_for_script;
>         osinfo_install_config_get_valid_params;
> diff --git a/osinfo/osinfo_db.c b/osinfo/osinfo_db.c
> index dcda2fe..eea8d12 100644
> --- a/osinfo/osinfo_db.c
> +++ b/osinfo/osinfo_db.c
> @@ -527,6 +527,77 @@ OsinfoOs *osinfo_db_guess_os_from_media(OsinfoDb *db,
>      return ret;
>  }
>
> +static void fill_media (OsinfoMedia *media, OsinfoMedia *matched_media, OsinfoOs *os)
> +{
> +    gboolean is_installer;
> +    gboolean is_live;
> +    gint reboots;
> +    const gchar *kernel_path;
> +    const gchar *initrd_path;
> +    const gchar *arch;
> +    const gchar *url;
> +
> +    arch = osinfo_media_get_architecture(matched_media);
> +    if (arch != NULL)
> +        g_object_set(G_OBJECT(media), "architecture", arch, NULL);
> +    url = osinfo_media_get_url(matched_media);
> +    if (url != NULL)
> +        g_object_set(G_OBJECT(media), "url", url, NULL);
> +
> +    kernel_path = osinfo_media_get_kernel_path(matched_media);
> +    if (kernel_path != NULL)
> +        g_object_set(G_OBJECT(media), "kernel_path", kernel_path, NULL);
> +
> +    initrd_path = osinfo_media_get_initrd_path(matched_media);
> +    if (initrd_path != NULL)
> +        g_object_set(G_OBJECT(media), "initrd_path", initrd_path, NULL);
> +    is_installer = osinfo_media_get_installer(matched_media);
> +    is_live = osinfo_media_get_live(matched_media);
> +    g_object_set(G_OBJECT(media),
> +                 "installer", is_installer,
> +                 "live", is_live,
> +                 NULL);
> +    if (is_installer) {
> +        reboots = osinfo_media_get_installer_reboots(matched_media);
> +        g_object_set(G_OBJECT(media), "installer-reboots", reboots, NULL);
> +    }
> +    if (os != NULL)
> +        osinfo_media_set_os(media, os);
> +}
> +
> +/**
> + * osinfo_db_identify_media:
> + * @db: a #OsinfoDb database
> + * @media: the installation media
> + * data
> + *
> + * Try to match the @media created using osinfo_media_create_from_location()
> + * with a media description from @db. If found, @media will be filled with
> + * the corresponding information stored in @db.

If we are going to keep media identifying API separate from media
creation, I suggest we don't make it sound like media must be created
using osinfo_media_create_from_location(). As you pointed out, we
could easily have other constructors in future. When that happens, its
very likely that we'll not remember to update this doc comment.

> In particular, after a call
> + * to osinfo_media_create_from_location(), if the media could be
> + * identified, its OsinfoMedia::os property will be set.

s/osinfo_media_create_from_location/osinfo_db_identify_media/ ? IMHO
the ", after a call to osinfo_media_create_from_location()," part is
quite redundant in this context.

> + * Returns: (transfer none): TRUE if @media was found in @db, FALSE

Not really an issue but for future ref: Transfer annotations are
redundant for basic C/glib types.

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124