Libguestfs - Aug 2014 - [PATCH] launch: direct: Only issue Debian /dev/kvm group warning if /dev/kvm wasn't openable (RHBZ#1130189).

This warning was meant for the case where /dev/kvm is inaccessible and
the user must add themselves to a special group.

However we didn't take into account whether /dev/kvm was openable in
this test.  If it's openable, no point issuing the warning.

Move the is_openable ("/dev/kvm") test earlier, and don't issue
the
warning if that was successful.
---
 src/launch-direct.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/launch-direct.c b/src/launch-direct.c
index 3bae254..3a8f2b2 100644
--- a/src/launch-direct.c
+++ b/src/launch-direct.c
@@ -289,11 +289,19 @@ 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___get_backend_setting_bool (g, "force_tcg");
   if (force_tcg == -1)
     return -1;
 
-  if (!force_tcg)
+  if (!has_kvm && !force_tcg)
     debian_kvm_warning (g);
 
   guestfs___launch_send_progress (g, 0);
@@ -411,14 +419,6 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
     warning (g, "qemu debugging is enabled, connect gdb to tcp::1234 to
begin");
   }
 
-  /* 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);
-
   cpu_model = guestfs___get_cpu_model (has_kvm && !force_tcg);
   if (cpu_model) {
     ADD_CMDLINE ("-cpu");
-- 
1.9.3

On Thursday 14 August 2014 15:18:10 Richard W.M. Jones
wrote:
> This warning was meant for the case where /dev/kvm is inaccessible and
> the user must add themselves to a special group.
> 
> However we didn't take into account whether /dev/kvm was openable in
> this test.  If it's openable, no point issuing the warning.
> 
> Move the is_openable ("/dev/kvm") test earlier, and don't
issue the
> warning if that was successful.
> ---
>  src/launch-direct.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/src/launch-direct.c b/src/launch-direct.c
> index 3bae254..3a8f2b2 100644
> --- a/src/launch-direct.c
> +++ b/src/launch-direct.c
> @@ -289,11 +289,19 @@ 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___get_backend_setting_bool (g,
"force_tcg");
>    if (force_tcg == -1)
>      return -1;
> 
> -  if (!force_tcg)
> +  if (!has_kvm && !force_tcg)
>      debian_kvm_warning (g);
> 
>    guestfs___launch_send_progress (g, 0);
> @@ -411,14 +419,6 @@ launch_direct (guestfs_h *g, void *datav, const
> char *arg)
>      warning (g, "qemu debugging is enabled, connect gdb to tcp::1234
>      to begin");
>    }
> 
> -  /* 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);
> -
>    cpu_model = guestfs___get_cpu_model (has_kvm && !force_tcg);
>    if (cpu_model) {
>      ADD_CMDLINE ("-cpu");
LGTM.

-- 
Pino Toscano