Libguestfs - Sep 2018 - [PATCH] lib: direct: Query qemu binary for availability of KVM (RHBZ#1605071).

When using the direct backend, you should see the result of testing
the qemu binary for the availability of KVM:

  libguestfs: qemu KVM: enabled

Thanks: Andrea Bologna.
---
 lib/guestfs-internal.h |  1 +
 lib/launch-direct.c    | 40 +++++-------------
 lib/qemu.c             | 94 +++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 105 insertions(+), 30 deletions(-)

diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h
index 841fd515b..f5eed82a1 100644
--- a/lib/guestfs-internal.h
+++ b/lib/guestfs-internal.h
@@ -777,6 +777,7 @@ extern struct version guestfs_int_qemu_version (guestfs_h
*g, struct qemu_data *
 extern int guestfs_int_qemu_supports (guestfs_h *g, const struct qemu_data *,
const char *option);
 extern int guestfs_int_qemu_supports_device (guestfs_h *g, const struct
qemu_data *, const char *device_name);
 extern int guestfs_int_qemu_mandatory_locking (guestfs_h *g, const struct
qemu_data *data);
+extern bool guestfs_int_platform_has_kvm (guestfs_h *g, const struct qemu_data
*data);
 extern char *guestfs_int_drive_source_qemu_param (guestfs_h *g, const struct
drive_source *src);
 extern bool guestfs_int_discard_possible (guestfs_h *g, struct drive *drv,
const struct version *qemu_version);
 extern char *guestfs_int_qemu_escape_param (guestfs_h *g, const char *param);
diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index e99c33347..40283d0d2 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -72,7 +72,6 @@ struct backend_direct_data {
   char guestfsd_sock[UNIX_PATH_MAX]; /* Path to daemon socket. */
 };
 
-static int is_openable (guestfs_h *g, const char *path, int flags);
 static char *make_appliance_dev (guestfs_h *g);
 
 static char *
@@ -387,21 +386,6 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
     return -1;
   }
 
-  /* Try to guess if KVM is available.  We are just checking that
-   * /dev/kvm is openable.  That's not reliable, since /dev/kvm
-   * might be openable by qemu but not by us (think: SELinux) in
-   * which case the user would not get hardware virtualization,
-   * although at least shouldn't fail.
-   */
-  has_kvm = is_openable (g, "/dev/kvm", O_RDWR|O_CLOEXEC);
-
-  force_tcg = guestfs_int_get_backend_setting_bool (g, "force_tcg");
-  if (force_tcg == -1)
-    return -1;
-
-  if (!has_kvm && !force_tcg)
-    debian_kvm_warning (g);
-
   guestfs_int_launch_send_progress (g, 0);
 
   TRACE0 (launch_build_appliance_start);
@@ -431,6 +415,17 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
            data->qemu_mandatory_locking ? "yes" : "no");
   }
 
+  /* Work out if KVM is supported or if the user wants to force TCG. */
+  has_kvm = guestfs_int_platform_has_kvm (g, data->qemu_data);
+  debug (g, "qemu KVM: %s", has_kvm ? "enabled" :
"disabled");
+
+  force_tcg = guestfs_int_get_backend_setting_bool (g, "force_tcg");
+  if (force_tcg == -1)
+    return -1;
+
+  if (!has_kvm && !force_tcg)
+    debian_kvm_warning (g);
+
   /* Using virtio-serial, we need to create a local Unix domain socket
    * for qemu to connect to.
    */
@@ -982,19 +977,6 @@ make_appliance_dev (guestfs_h *g)
   return safe_strdup (g, dev);  /* Caller frees. */
 }
 
-/* Check if a file can be opened. */
-static int
-is_openable (guestfs_h *g, const char *path, int flags)
-{
-  int fd = open (path, flags);
-  if (fd == -1) {
-    debug (g, "is_openable: %s: %m", path);
-    return 0;
-  }
-  close (fd);
-  return 1;
-}
-
 static int
 shutdown_direct (guestfs_h *g, void *datav, int check_for_errors)
 {
diff --git a/lib/qemu.c b/lib/qemu.c
index 3e7f15946..212cda963 100644
--- a/lib/qemu.c
+++ b/lib/qemu.c
@@ -46,6 +46,19 @@
 #include "guestfs-internal.h"
 #include "guestfs_protocol.h"
 
+#ifdef HAVE_ATTRIBUTE_CLEANUP
+#define CLEANUP_JSON_T_DECREF __attribute__((cleanup(cleanup_json_t_decref)))
+
+static void
+cleanup_json_t_decref (void *ptr)
+{
+  json_decref (* (json_t **) ptr);
+}
+
+#else
+#define CLEANUP_JSON_T_DECREF
+#endif
+
 struct qemu_data {
   int generation;               /* MEMO_GENERATION read from qemu.stat */
   uint64_t prev_size;           /* Size of qemu binary when cached. */
@@ -54,10 +67,12 @@ struct qemu_data {
   char *qemu_help;              /* Output of qemu -help. */
   char *qemu_devices;           /* Output of qemu -device ? */
   char *qmp_schema;             /* Output of QMP query-qmp-schema. */
+  char *query_kvm;              /* Output of QMP query-kvm. */
 
   /* The following fields are derived from the fields above. */
   struct version qemu_version;  /* Parsed qemu version number. */
   json_t *qmp_schema_tree;      /* qmp_schema parsed into a JSON tree */
+  bool has_kvm;                 /* If KVM is available. */
 };
 
 static char *cache_filename (guestfs_h *g, const char *cachedir, const struct
stat *, const char *suffix);
@@ -70,10 +85,14 @@ static int write_cache_qemu_devices (guestfs_h *g, const
struct qemu_data *data,
 static int test_qmp_schema (guestfs_h *g, struct qemu_data *data);
 static int read_cache_qmp_schema (guestfs_h *g, struct qemu_data *data, const
char *filename);
 static int write_cache_qmp_schema (guestfs_h *g, const struct qemu_data *data,
const char *filename);
+static int test_query_kvm (guestfs_h *g, struct qemu_data *data);
+static int read_cache_query_kvm (guestfs_h *g, struct qemu_data *data, const
char *filename);
+static int write_cache_query_kvm (guestfs_h *g, const struct qemu_data *data,
const char *filename);
 static int read_cache_qemu_stat (guestfs_h *g, struct qemu_data *data, const
char *filename);
 static int write_cache_qemu_stat (guestfs_h *g, const struct qemu_data *data,
const char *filename);
 static void parse_qemu_version (guestfs_h *g, const char *, struct version
*qemu_version);
 static void parse_json (guestfs_h *g, const char *, json_t **);
+static void parse_has_kvm (guestfs_h *g, const char *, bool *);
 static void read_all (guestfs_h *g, void *retv, const char *buf, size_t len);
 static int generic_read_cache (guestfs_h *g, const char *filename, char
**strp);
 static int generic_write_cache (guestfs_h *g, const char *filename, const char
*str);
@@ -105,6 +124,8 @@ static const struct qemu_fields {
     test_qemu_devices, read_cache_qemu_devices, write_cache_qemu_devices },
   { "qmp-schema",
     test_qmp_schema, read_cache_qmp_schema, write_cache_qmp_schema },
+  { "query-kvm",
+    test_query_kvm, read_cache_query_kvm, write_cache_query_kvm },
 };
 #define NR_FIELDS (sizeof qemu_fields / sizeof qemu_fields[0])
 
@@ -113,7 +134,7 @@ static const struct qemu_fields {
  * this to discard any memoized data cached by previous versions of
  * libguestfs.
  */
-#define MEMO_GENERATION 2
+#define MEMO_GENERATION 3
 
 /**
  * Test that the qemu binary (or wrapper) runs, and do C<qemu -help>
@@ -211,6 +232,7 @@ guestfs_int_test_qemu (guestfs_h *g)
   /* Derived fields. */
   parse_qemu_version (g, data->qemu_help, &data->qemu_version);
   parse_json (g, data->qmp_schema, &data->qmp_schema_tree);
+  parse_has_kvm (g, data->query_kvm, &data->has_kvm);
 
   return data;
 
@@ -338,6 +360,26 @@ write_cache_qmp_schema (guestfs_h *g, const struct
qemu_data *data,
   return generic_write_cache (g, filename, data->qmp_schema);
 }
 
+static int
+test_query_kvm (guestfs_h *g, struct qemu_data *data)
+{
+  return generic_qmp_test (g, data, "query-kvm",
&data->query_kvm);
+}
+
+static int
+read_cache_query_kvm (guestfs_h *g, struct qemu_data *data,
+                      const char *filename)
+{
+  return generic_read_cache (g, filename, &data->query_kvm);
+}
+
+static int
+write_cache_query_kvm (guestfs_h *g, const struct qemu_data *data,
+                       const char *filename)
+{
+  return generic_write_cache (g, filename, data->query_kvm);
+}
+
 static int
 read_cache_qemu_stat (guestfs_h *g, struct qemu_data *data,
                       const char *filename)
@@ -421,6 +463,49 @@ parse_json (guestfs_h *g, const char *json, json_t **treep)
   }
 }
 
+/**
+ * Parse the json output from QMP query-kvm to find out if KVM is
+ * enabled on this machine.  Don't fail if parsing is not possible,
+ * assume KVM is available.
+ *
+ * The JSON output looks like:
+ * {"return": {"enabled": true, "present": true}}
+ */
+static void
+parse_has_kvm (guestfs_h *g, const char *json, bool *ret)
+{
+  CLEANUP_JSON_T_DECREF json_t *tree = NULL;
+  json_error_t err;
+  json_t *return_node, *enabled_node;
+
+  *ret = true;                  /* Assume KVM is enabled. */
+
+  if (!json)
+    return;
+
+  tree = json_loads (json, 0, &err);
+  if (tree == NULL) {
+    if (strlen (err.text) > 0)
+      debug (g, "QMP parse error: %s (ignored)", err.text);
+    else
+      debug (g, "QMP unknown parse error (ignored)");
+    return;
+  }
+
+  return_node = json_object_get (tree, "return");
+  if (!json_is_object (return_node)) {
+    debug (g, "QMP query-kvm: no \"return\" node
(ignored)");
+    return;
+  }
+  enabled_node = json_object_get (return_node, "enabled");
+  if (!enabled_node || !json_is_boolean (enabled_node)) {
+    debug (g, "QMP query-kvm: no \"enabled\" node or not a
boolean (ignored)");
+    return;
+  }
+
+  *ret = json_is_true (enabled_node);
+}
+
 /**
  * Generic functions for reading and writing the cache files, used
  * where we are just reading and writing plain text strings.
@@ -628,6 +713,12 @@ guestfs_int_qemu_mandatory_locking (guestfs_h *g,
   return 0;
 }
 
+bool
+guestfs_int_platform_has_kvm (guestfs_h *g, const struct qemu_data *data)
+{
+  return data->has_kvm;
+}
+
 /**
  * Escape a qemu parameter.
  *
@@ -976,6 +1067,7 @@ guestfs_int_free_qemu_data (struct qemu_data *data)
     free (data->qemu_help);
     free (data->qemu_devices);
     free (data->qmp_schema);
+    free (data->query_kvm);
     json_decref (data->qmp_schema_tree);
     free (data);
   }
-- 
2.19.0.rc0

On Thursday, 13 September 2018 13:29:49 CEST Richard W.M. Jones
wrote:
> When using the direct backend, you should see the result of testing
> the qemu binary for the availability of KVM:
> 
>   libguestfs: qemu KVM: enabled
> 
> Thanks: Andrea Bologna.
typo: "Andrea Bolognani"
> +  enabled_node = json_object_get (return_node, "enabled");
> +  if (!enabled_node || !json_is_boolean (enabled_node)) {
No need to check !enabled_node, all the various json_is_* functions
handle NULL values already.

With the above fixes, LGTM.

-- 
Pino Toscano