[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [virt-tools-list] [PATCH libosinfo 1/3] Add support for install tree metadata



On Thu, Feb 23, 2012 at 3:27 PM, Daniel P. Berrange <berrange redhat com> wrote:
> On Thu, Feb 23, 2012 at 03:15:36PM +0200, Zeeshan Ali (Khattak) wrote:
>> On Thu, Feb 23, 2012 at 2:23 PM, Daniel P. Berrange <berrange redhat com> wrote:
>> > From: "Daniel P. Berrange" <berrange redhat com>
>> >
>> > The <media> element and OsinfoMedia class can be used to identify
>> > install media, ie ISO images.
>> >
>> > The <tree> element and OsinfoTree class are the same concept but
>> > used to identify installation trees.
>>
>> Looks good. Some comments:
>>
>> > diff --git a/osinfo/osinfo_db.c b/osinfo/osinfo_db.c
>> > index ecc8fbd..986e7c9 100644
>> > --- a/osinfo/osinfo_db.c
>> > +++ b/osinfo/osinfo_db.c
>> > @@ -31,6 +31,7 @@ G_DEFINE_TYPE (OsinfoDb, osinfo_db, G_TYPE_OBJECT);
>> >  #define OSINFO_DB_GET_PRIVATE(obj) (G_TYPE_INSTANCE_GET_PRIVATE ((obj), OSINFO_TYPE_DB, OsinfoDbPrivate))
>> >
>> >  #define match_regex(pattern, str) (((pattern) == NULL && (str) == NULL) || \
>> > +                                   ((pattern) == NULL) ||               \
>> >                                    ((pattern) != NULL && (str) != NULL && \
>> >                                     g_regex_match_simple((pattern), (str), 0, 0)))
>> >
>> > @@ -386,11 +387,10 @@ OsinfoOs *osinfo_db_guess_os_from_media(OsinfoDb *db,
>> >             const gchar *os_publisher = osinfo_media_get_publisher_id(os_media);
>> >             const gchar *os_application = osinfo_media_get_application_id(os_media);
>> >
>> > -            if ((match_regex (os_volume, media_volume) ||
>> > -                 match_regex (os_application, media_application))
>> > -                 &&
>> > -                (match_regex (os_system, media_system) ||
>> > -                 match_regex (os_publisher, media_publisher))) {
>> > +            if (match_regex (os_volume, media_volume) &&
>> > +                match_regex (os_application, media_application) &&
>> > +                match_regex (os_system, media_system) &&
>> > +                match_regex (os_publisher, media_publisher)) {
>> >                 ret = os;
>> >                 if (matched_media != NULL)
>> >                     *matched_media = os_media;
>> > @@ -410,6 +410,77 @@ OsinfoOs *osinfo_db_guess_os_from_media(OsinfoDb *db,
>> >     return ret;
>> >  }
>>
>> I guess this got included accidently?
>
>
> Yeah, I split this off into the separate commit I posted earlier, and
> forgot to remove it here.
>
>
>>
>> > +static OsinfoTree *load_keyinfo(const gchar *location,
>> > +                                const gchar *content,
>> > +                                gsize length,
>> > +                                GError **error)
>> > +{
>> > +    GKeyFile *file = g_key_file_new();
>> > +    OsinfoTree *tree = NULL;
>> > +    gchar *family = NULL;
>> > +    gchar *variant = NULL;
>> > +    gchar *version = NULL;
>> > +    gchar *arch = NULL;
>> > +    gchar *kernel = NULL;
>> > +    gchar *initrd = NULL;
>> > +    gchar *bootiso = NULL;
>> > +    gchar *group = NULL;
>> > +
>> > +    if (!g_key_file_load_from_data(file, content, length,
>> > +                                   G_KEY_FILE_NONE, error))
>> > +        goto cleanup;
>> > +
>> > +    if (!(family = g_key_file_get_string(file, "general", "family", error)) &&
>> > +        (*error && (*error)->code != G_KEY_FILE_ERROR_KEY_NOT_FOUND &&
>> > +         (*error)->code != G_KEY_FILE_ERROR_GROUP_NOT_FOUND))
>> > +        goto cleanup;
>> > +
>> > +    if (!(variant = g_key_file_get_string(file, "general", "variant", error)) &&
>> > +        (*error && (*error)->code != G_KEY_FILE_ERROR_KEY_NOT_FOUND &&
>> > +         (*error)->code != G_KEY_FILE_ERROR_GROUP_NOT_FOUND))
>> > +        goto cleanup;
>> > +
>> > +    if (!(version = g_key_file_get_string(file, "general", "version", error)) &&
>> > +        (*error && (*error)->code != G_KEY_FILE_ERROR_KEY_NOT_FOUND &&
>> > +         (*error)->code != G_KEY_FILE_ERROR_GROUP_NOT_FOUND))
>> > +        goto cleanup;
>> > +
>> > +    if (!(arch = g_key_file_get_string(file, "general", "arch", error)) &&
>> > +        (*error && (*error)->code != G_KEY_FILE_ERROR_KEY_NOT_FOUND &&
>> > +         (*error)->code != G_KEY_FILE_ERROR_GROUP_NOT_FOUND))
>> > +        goto cleanup;
>>
>> I don't this part. Isn't the location supposed to be some remote
>> directory? Where does a key file come from?
>
> We pass a URI to GFile and load that into memory, using its builtin http
> support. So when we use the GKeyFile, we're just giving it a pre-loaded
> chunk of memory, thus avoiding the problem that it only understands local
> file paths.

No, I understood the loading part. I just didn't get where you get the
key file/data from given a URI of install tree?

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]