[virt-tools-list] [libosinfo 2/2] Correct & further restrict media matching logic

Daniel P. Berrange berrange at redhat.com
Thu Feb 23 11:12:44 UTC 2012


On Thu, Feb 23, 2012 at 02:00:42AM +0200, Zeeshan Ali (Khattak) wrote:
> From: "Zeeshan Ali (Khattak)" <zeeshanak at gnome.org>
> 
> Recently we started to use application IDs for detection, which is
> itself a good change but we ended-up turning the matching logic a lot
> more liberal than it should be. This patches fixes that and makes the
> application ID match compulsory.
> ---
>  osinfo/osinfo_db.c |    9 ++++-----
>  1 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/osinfo/osinfo_db.c b/osinfo/osinfo_db.c
> index ecc8fbd..d1eba9f 100644
> --- a/osinfo/osinfo_db.c
> +++ b/osinfo/osinfo_db.c
> @@ -386,11 +386,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_system, media_system) &&
> +                match_regex (os_application, media_application) &&
> +                match_regex (os_publisher, media_publisher)) {
>                  ret = os;
>                  if (matched_media != NULL)
>                      *matched_media = os_media;

This is going in the right direction, but IMHO it is not complete. With
this, it is neccessary to fully specify all fields in the OSinfo database
even if they are not really neccessary for a proper match. IMHO we need
to also fix the 'match_regex' macro logic.

With the alternative patch I just sent, then we don't need to add in all
that CDIMAGE* stuff in your first patch.

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




More information about the virt-tools-list mailing list