Libguestfs - Jul 2016 - [PATCH] osinfo: do not assume every media is an installer

Small cleanup helper to dispose xmlChar* buffers.
---
 src/cleanup.c                   | 9 +++++++++
 src/guestfs-internal-frontend.h | 4 ++++
 2 files changed, 13 insertions(+)

diff --git a/src/cleanup.c b/src/cleanup.c
index 1aa3051..6c4558c 100644
--- a/src/cleanup.c
+++ b/src/cleanup.c
@@ -106,6 +106,15 @@ guestfs_int_cleanup_unlink_free (char **ptr)
 }
 
 void
+guestfs_int_cleanup_xmlFree (void *ptr)
+{
+  xmlChar *buf = * (xmlChar **) ptr;
+
+  if (buf)
+    xmlFree (buf);
+}
+
+void
 guestfs_int_cleanup_xmlBufferFree (void *ptr)
 {
   xmlBufferPtr xb = * (xmlBufferPtr *) ptr;
diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h
index d1de76d..8689009 100644
--- a/src/guestfs-internal-frontend.h
+++ b/src/guestfs-internal-frontend.h
@@ -43,6 +43,8 @@
   __attribute__((cleanup(guestfs_int_cleanup_hash_free)))
 #define CLEANUP_UNLINK_FREE                                     \
   __attribute__((cleanup(guestfs_int_cleanup_unlink_free)))
+#define CLEANUP_XMLFREE                                         \
+  __attribute__((cleanup(guestfs_int_cleanup_xmlFree)))
 #define CLEANUP_XMLBUFFERFREE                                   \
   __attribute__((cleanup(guestfs_int_cleanup_xmlBufferFree)))
 #define CLEANUP_XMLFREEDOC                                      \
@@ -62,6 +64,7 @@
 #define CLEANUP_FREE_STRING_LIST
 #define CLEANUP_HASH_FREE
 #define CLEANUP_UNLINK_FREE
+#define CLEANUP_XMLFREE
 #define CLEANUP_XMLBUFFERFREE
 #define CLEANUP_XMLFREEDOC
 #define CLEANUP_XMLFREEURI
@@ -109,6 +112,7 @@ extern void guestfs_int_cleanup_free (void *ptr);
 extern void guestfs_int_cleanup_free_string_list (char ***ptr);
 extern void guestfs_int_cleanup_hash_free (void *ptr);
 extern void guestfs_int_cleanup_unlink_free (char **ptr);
+extern void guestfs_int_cleanup_xmlFree (void *ptr);
 extern void guestfs_int_cleanup_xmlBufferFree (void *ptr);
 extern void guestfs_int_cleanup_xmlFreeDoc (void *ptr);
 extern void guestfs_int_cleanup_xmlFreeURI (void *ptr);
-- 
2.7.4

Directly query the attributes in the media node, instead of using an
XPath expression to read it.

Saves ~10ms when parsing the libosinfo DB on a Fedora 24 installation
(other than making the code easier).
---
 src/osinfo.c | 30 ++++++++----------------------
 1 file changed, 8 insertions(+), 22 deletions(-)

diff --git a/src/osinfo.c b/src/osinfo.c
index a5026f3..22f1e92 100644
--- a/src/osinfo.c
+++ b/src/osinfo.c
@@ -74,6 +74,8 @@ static struct osinfo *osinfo_db = NULL;
 static int read_osinfo_db (guestfs_h *g);
 static void free_osinfo_db_entry (struct osinfo *);
 
+#define XMLSTREQ(a,b) (xmlStrEqual((a),(b)) == 1)
+
 /* Given one or more fields from the header of a CD/DVD/ISO, look up
  * the media in the libosinfo database and return our best guess for
  * the operating system.
@@ -483,30 +485,14 @@ static int
 read_media_node (guestfs_h *g, xmlXPathContextPtr xpathCtx,
                  xmlNodePtr media_node, struct osinfo *osinfo)
 {
-  CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xp = NULL, xp2 = NULL;
-  xmlNodePtr attr;
-
-  xpathCtx->node = media_node;
-  xp = xmlXPathEvalExpression (BAD_CAST "./@arch", xpathCtx);
-  if (xp && xp->nodesetval && xp->nodesetval->nodeNr
> 0) {
-    attr = xp->nodesetval->nodeTab[0];
-    assert (attr);
-    assert (attr->type == XML_ATTRIBUTE_NODE);
-    osinfo->arch = (char *) xmlNodeGetContent (attr);
-  }
+  osinfo->arch = (char *) xmlGetProp (media_node, BAD_CAST
"arch");
 
   osinfo->is_live_disk = 0; /* If no 'live' attr, defaults to false.
*/
-
-  xpathCtx->node = media_node;
-  xp2 = xmlXPathEvalExpression (BAD_CAST "./@live", xpathCtx);
-  if (xp2 && xp2->nodesetval &&
xp2->nodesetval->nodeNr > 0) {
-    CLEANUP_FREE char *content = NULL;
-
-    attr = xp2->nodesetval->nodeTab[0];
-    assert (attr);
-    assert (attr->type == XML_ATTRIBUTE_NODE);
-    content = (char *) xmlNodeGetContent (attr);
-    osinfo->is_live_disk = STREQ (content, "true");
+  {
+    CLEANUP_XMLFREE xmlChar *content = NULL;
+    content = xmlGetProp (media_node, BAD_CAST "live");
+    if (content)
+      osinfo->is_live_disk = XMLSTREQ (content, BAD_CAST "true");
   }
 
   return 0;
-- 
2.7.4

The osinfo has an "installer" attribute in <media>, which
defaults to
true, and there are few entries which are marked as not.
---
 src/guestfs-internal.h |  1 +
 src/inspect-fs-cd.c    |  3 ++-
 src/osinfo.c           | 11 ++++++++++-
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 03f1034..2b49011 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -861,6 +861,7 @@ struct osinfo {
   int minor_version;
   char *arch;
   int is_live_disk;
+  bool is_installer;
 
 #if 0
   /* Not yet available in libosinfo database. */
diff --git a/src/inspect-fs-cd.c b/src/inspect-fs-cd.c
index bb28c0f..10e9d54 100644
--- a/src/inspect-fs-cd.c
+++ b/src/inspect-fs-cd.c
@@ -526,7 +526,8 @@ guestfs_int_check_installer_iso (guestfs_h *g, struct
inspect_fs *fs,
   /* Otherwise we matched an ISO, so fill in the fs fields. */
   fs->mountable = safe_strdup (g, device);
   fs->is_root = 1;
-  fs->format = OS_FORMAT_INSTALLER;
+  if (osinfo->is_installer)
+    fs->format = OS_FORMAT_INSTALLER;
   fs->type = osinfo->type;
   fs->distro = osinfo->distro;
   fs->product_name diff --git a/src/osinfo.c b/src/osinfo.c
index 22f1e92..fc18075 100644
--- a/src/osinfo.c
+++ b/src/osinfo.c
@@ -412,7 +412,7 @@ read_osinfo_db_xml (guestfs_h *g, const char *pathname)
       }
 
 #if 0
-      debug (g, "osinfo: %s: %s%s%s%s=> arch %s live %s product %s type
%d distro %d version %d.%d",
+      debug (g, "osinfo: %s: %s%s%s%s=> arch %s live %s installer %s
product %s type %d distro %d version %d.%d",
              pathname,
              osinfo->re_system_id ? "<system-id/> " :
"",
              osinfo->re_volume_id ? "<volume-id/> " :
"",
@@ -420,6 +420,7 @@ read_osinfo_db_xml (guestfs_h *g, const char *pathname)
              osinfo->re_application_id ? "<application-id/>
" : "",
              osinfo->arch ? osinfo->arch : "(none)",
              osinfo->is_live_disk ? "true" : "false",
+             osinfo->is_installer ? "true" : "false",
              osinfo->product_name ? osinfo->product_name :
"(none)",
              (int) osinfo->type, (int) osinfo->distro,
              osinfo->major_version, osinfo->minor_version);
@@ -495,6 +496,14 @@ read_media_node (guestfs_h *g, xmlXPathContextPtr xpathCtx,
       osinfo->is_live_disk = XMLSTREQ (content, BAD_CAST "true");
   }
 
+  osinfo->is_installer = true; /* If no 'installer' attr, defaults
to true. */
+  {
+    CLEANUP_XMLFREE xmlChar *content = NULL;
+    content = xmlGetProp (media_node, BAD_CAST "installer");
+    if (content)
+      osinfo->is_installer = XMLSTREQ (content, BAD_CAST "true");
+  }
+
   return 0;
 }
 
-- 
2.7.4

On Thu, Jul 28, 2016 at 04:21:10PM +0200, Pino Toscano
wrote:
> The osinfo has an "installer" attribute in <media>, which
defaults to
> true, and there are few entries which are marked as not.
> ---
>  src/guestfs-internal.h |  1 +
>  src/inspect-fs-cd.c    |  3 ++-
>  src/osinfo.c           | 11 ++++++++++-
>  3 files changed, 13 insertions(+), 2 deletions(-)
ACK all 3 patches.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/