[libvirt] [libvirt-designer PATCHv2] Add GvirDesignerDomain::osinfo_db

[Daniel P. Berrange]

On Wed, Mar 20, 2013 at 11:10:50AM +0100, Christophe Fergeau wrote:
> virtxml was doing its own loading of the libosinfo database,
> and gvir_designer_init() was loading it a second time.
> By adding a GVirDesignerDomain::osinfo_db property, applications
> can share the same OsinfoDb as libvirt-designer. The association
> is made per libvirt-designer domain for more flexibility.
> If no OsinfoDb is associated with a domain when needed,
> libvirt-designer will automatically try to load it.
> ---
>  examples/virtxml.c                           |  2 ++
>  libvirt-designer/libvirt-designer-domain.c   | 49 +++++++++++++++++++++++++++-
>  libvirt-designer/libvirt-designer-internal.h |  3 --
>  libvirt-designer/libvirt-designer-main.c     | 12 -------
>  libvirt-designer/libvirt-designer-main.h     |  2 ++
>  libvirt-designer/libvirt-designer.sym        |  1 +
>  6 files changed, 53 insertions(+), 16 deletions(-)
> 
> diff --git a/examples/virtxml.c b/examples/virtxml.c
> index a68843d..be6ee7a 100644
> --- a/examples/virtxml.c
> +++ b/examples/virtxml.c
> @@ -559,6 +559,8 @@ main(int argc, char *argv[])
>      }
>  
>      domain = gvir_designer_domain_new(os, platform, caps);
> +    if (db != NULL)
> +        g_object_set(G_OBJECT(domain), "osinfo-db", db, NULL);

Can't we just pass this into the constructor. I know it is
an API change, but I don't really consider libvirt-designer
to be at a point in its development where we consider it to
be ABI stable.

>  
> +static void gvir_designer_domain_load_osinfo_db(GVirDesignerDomain *domain)
> +{
> +    /* Init libosinfo and load databases from default paths */
> +    /* XXX maybe we want to let users tell a different path via
> +     * env variable or argv */
> +    OsinfoLoader *osinfo_loader = NULL;
> +    OsinfoDb *db;
> +
> +    osinfo_loader = osinfo_loader_new();
> +    osinfo_loader_process_default_path(osinfo_loader, NULL);
> +
> +    db = osinfo_loader_get_db(osinfo_loader);
> +    if (db != NULL)
> +        g_object_set(G_OBJECT(domain), "osinfo-db", db, NULL);
> +    g_object_unref(G_OBJECT(osinfo_loader));
> +}

I don';t think we should be doing this - we should consider
the "db" parameter to be mandatory for the application to
provided, and hte app should make sure it is preloaded.

>  static OsinfoDeviceLink *
>  gvir_designer_domain_get_preferred_device(GVirDesignerDomain *design,
>                                            const char *class,
> @@ -721,7 +760,15 @@ gvir_designer_domain_get_preferred_device(GVirDesignerDomain *design,
>      OsinfoDeviceLink *dev_link = NULL;
>  
>      if (!deployment) {
> -        priv->deployment = deployment = osinfo_db_find_deployment(osinfo_db,
> +        if (!priv->osinfo_db)
> +            gvir_designer_domain_load_osinfo_db(design);
> +
> +        if (!priv->osinfo_db) {
> +            g_set_error(error, GVIR_DESIGNER_DOMAIN_ERROR, 0,
> +                        "Unable to find any deployment in libosinfo database");
> +            goto cleanup;
> +        }
> +        priv->deployment = deployment = osinfo_db_find_deployment(priv->osinfo_db,
>                                                                    priv->os,
>                                                                    priv->platform);
>          if (!deployment) {
> diff --git a/libvirt-designer/libvirt-designer.sym b/libvirt-designer/libvirt-designer.sym
> index 79db09f..6f63abc 100644
> --- a/libvirt-designer/libvirt-designer.sym
> +++ b/libvirt-designer/libvirt-designer.sym
> @@ -2,6 +2,7 @@ LIBVIRT_DESIGNER_0.0.1 {
>     global:
>  	gvir_designer_init;
>  	gvir_designer_init_check;
> +	gvir_designer_get_osinfo_db;

Left over from previous patch

>  
>  	gvir_designer_domain_new;
>  	gvir_designer_domain_get_type;


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