[libvirt] [libvirt-designer][PATCH v2 1/4] Load osinfo DB on init

[Michal Privoznik]

On 12.09.2012 11:19, Christophe Fergeau wrote:
> Hey,
> 
> I should have looked more in depth at these commits earlier, sorry for the
> late feedback ;)

Better late than never :)

> 
> On Mon, Sep 10, 2012 at 03:58:25PM +0200, Michal Privoznik wrote:
>> diff --git a/libvirt-designer/libvirt-designer-internal.h b/libvirt-designer/libvirt-designer-internal.h
>> new file mode 100644
>> index 0000000..bbef922
>> --- /dev/null
>> +++ b/libvirt-designer/libvirt-designer-internal.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * libvirt-designer-internal.h: internal definitions just
>> + *                              used by code from the library
>> + *
>> + * Copyright (C) 2012 Red Hat, Inc.
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; If not, see
>> + * <http://www.gnu.org/licenses/>.
>> + *
>> + * Author: Michal Privoznik <mprivozn at redhat.com>
>> + */
>> +
>> +#ifndef __LIBVIRT_DESIGNER_INTERNAL_H__
>> +#define __LIBVIRT_DESIGNER_INTERNAL_H__
>> +
>> +extern OsinfoLoader *osinfo_loader;
>> +extern OsinfoDb *osinfo_db;
>> +
>> +#endif /* __LIBVIRT_DESIGNER_INTERNAL_H__ */
>> diff --git a/libvirt-designer/libvirt-designer-main.c b/libvirt-designer/libvirt-designer-main.c
>> index 60bf8f5..f2381a6 100644
>> --- a/libvirt-designer/libvirt-designer-main.c
>> +++ b/libvirt-designer/libvirt-designer-main.c
>> @@ -17,7 +17,9 @@
>>   * License along with this library; If not, see
>>   * <http://www.gnu.org/licenses/>.
>>   *
>> - * Author: Daniel P. Berrange <berrange at redhat.com>
>> + * Authors:
>> + *   Daniel P. Berrange <berrange at redhat.com>
>> + *   Michal Privoznik <mprivozn at redhat.com>
>>   */
>>  
>>  #include <config.h>
>> @@ -28,6 +30,9 @@
>>  #include <libvirt-designer/libvirt-designer.h>
>>  #include <libvirt-gconfig/libvirt-gconfig.h>
>>  
>> +OsinfoLoader *osinfo_loader = NULL;
>> +OsinfoDb *osinfo_db = NULL;
>> +
>>  /**
>>   * gvir_designer_init:
>>   * @argc: (inout): pointer to application's argc
>> @@ -80,5 +85,15 @@ gboolean gvir_designer_init_check(int *argc,
>>                            gvir_log_handler, NULL);
>>  #endif
>>  
>> +    /* Init libosinfo and load databases from default paths */
>> +    /* XXX maybe we want to let users tell a different path via
>> +     * env variable or argv */
> 
> osinfo_loader_process_default_path() looks into
> $XDG_CONFIG_DIR/libosinfo/db so end users can override it, but users of the
> library may indeed also want to be able to override it.

Yeah; for now I just call process_default_path() however in future users
may with to process just a specific file, path, whatever. Don't know if
it is something that should be passed through argv[] or an API. I guess
we will decide if such moment come.

> 
> 
>> +    osinfo_loader = osinfo_loader_new();
>> +    osinfo_loader_process_default_path(osinfo_loader, err);
>> +    if (err)
>> +        return FALSE;
> 
> I'm not sure we want to error out there, see
> http://git.fedorahosted.org/cgit/libosinfo.git/commit/?id=dbde512c3a64640d61fa5e7f801050e248f60c98
> for my reasoning (and you can try a "mkdir -p ~/.config/libosinfo/db &&
> echo '<bad' >~/.config/libosinfo/db/broken.xml" to get into a 'bad'
> situation).

IIUC, malformed XML doesn't affect loading of other well-formed XMLs,
right? If this is the case, then yes - we should not report error.
However, if malformed XML results in empty libosinfo DB, then we must
report error here as non-empty DB is crucial for libvirt-designer.

Michal

> 
> Christophe
>