[PATCH v1] tools: add virt-host-validate-ch for ch driver

Praveen K Paladugu posted 1 patch 2 years, 6 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20211007214937.1716538-1-prapal@linux.microsoft.com
po/POTFILES.in                |  1 +
tools/meson.build             |  5 +++
tools/virt-host-validate-ch.c | 85 +++++++++++++++++++++++++++++++++++
tools/virt-host-validate-ch.h | 24 ++++++++++
tools/virt-host-validate.c    | 12 +++++
5 files changed, 127 insertions(+)
create mode 100644 tools/virt-host-validate-ch.c
create mode 100644 tools/virt-host-validate-ch.h
[PATCH v1] tools: add virt-host-validate-ch for ch driver
Posted by Praveen K Paladugu 2 years, 6 months ago
Signed-off-by: Wei-Chen Chen <weicche@microsoft.com>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
---
 po/POTFILES.in                |  1 +
 tools/meson.build             |  5 +++
 tools/virt-host-validate-ch.c | 85 +++++++++++++++++++++++++++++++++++
 tools/virt-host-validate-ch.h | 24 ++++++++++
 tools/virt-host-validate.c    | 12 +++++
 5 files changed, 127 insertions(+)
 create mode 100644 tools/virt-host-validate-ch.c
 create mode 100644 tools/virt-host-validate-ch.h

diff --git a/po/POTFILES.in b/po/POTFILES.in
index c200d7452a..b554cf08ca 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -369,6 +369,7 @@
 @SRCDIR@tools/virsh.h
 @SRCDIR@tools/virt-admin.c
 @SRCDIR@tools/virt-host-validate-bhyve.c
+@SRCDIR@tools/virt-host-validate-ch.c
 @SRCDIR@tools/virt-host-validate-common.c
 @SRCDIR@tools/virt-host-validate-lxc.c
 @SRCDIR@tools/virt-host-validate-qemu.c
diff --git a/tools/meson.build b/tools/meson.build
index 2acf7b0aaf..bf0eab8b6b 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -58,6 +58,11 @@ if conf.has('WITH_HOST_VALIDATE')
       'virt-host-validate-bhyve.c',
     ]
   endif
+  if conf.has('WITH_CH')
+    virt_host_validate_sources += [
+      'virt-host-validate-ch.c',
+    ]
+  endif
 
   executable(
     'virt-host-validate',
diff --git a/tools/virt-host-validate-ch.c b/tools/virt-host-validate-ch.c
new file mode 100644
index 0000000000..a6d8a01d1b
--- /dev/null
+++ b/tools/virt-host-validate-ch.c
@@ -0,0 +1,85 @@
+/*
+ * Copyright Microsoft Corp. 2020-2021
+ *
+ * virt-host-validate-ch.c: Sanity check a CH hypervisor host
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+#include <config.h>
+
+#include "virarch.h"
+#include "virbitmap.h"
+#include "virt-host-validate-ch.h"
+#include "virt-host-validate-common.h"
+
+int virHostValidateCh(void)
+{
+  int ret = 0;
+  virBitmap *flags;
+  bool hasHwVirt = false;
+  bool hasVirtFlag = false;
+  virArch arch = virArchFromHost();
+  const char *kvmhint =
+      _("Check that CPU and firmware supports virtualization "
+        "and kvm module is loaded");
+
+  if (!(flags = virHostValidateGetCPUFlags()))
+    return -1;
+
+  // Cloud-Hypervisor only supports x86_64 and aarch64
+  switch ((int)arch) {
+    case VIR_ARCH_X86_64:
+      hasVirtFlag = true;
+      kvmhint = _("Check that the 'kvm-intel' or 'kvm-amd' modules are "
+                  "loaded & the BIOS has enabled virtualization");
+      if (virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_SVM) ||
+          virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_VMX))
+        hasHwVirt = true;
+      break;
+    case VIR_ARCH_AARCH64:
+      hasVirtFlag = true;
+      hasHwVirt = true;
+      break;
+    default:
+      hasHwVirt = false;
+      break;
+  }
+
+  if (hasVirtFlag) {
+    virHostMsgCheck("CH", "%s", _("for hardware virtualization"));
+    if (hasHwVirt) {
+      virHostMsgPass();
+    } else {
+      virHostMsgFail(VIR_HOST_VALIDATE_FAIL,
+                     _("Only emulated CPUs are available, performance will be "
+                       "significantly limited"));
+      ret = -1;
+    }
+  }
+
+  if (hasHwVirt || !hasVirtFlag) {
+    if (virHostValidateDeviceExists("CH", "/dev/kvm", VIR_HOST_VALIDATE_FAIL,
+                                    kvmhint) < 0)
+      ret = -1;
+    else if (virHostValidateDeviceAccessible(
+                 "CH", "/dev/kvm", VIR_HOST_VALIDATE_FAIL,
+                 _("Check /dev/kvm is world writable or you are in "
+                   "a group that is allowed to access it")) < 0)
+      ret = -1;
+  }
+
+  return ret;
+}
diff --git a/tools/virt-host-validate-ch.h b/tools/virt-host-validate-ch.h
new file mode 100644
index 0000000000..b16e7d36ab
--- /dev/null
+++ b/tools/virt-host-validate-ch.h
@@ -0,0 +1,24 @@
+/*
+ * virt-host-validate-ch.h: Sanity check a CH hypervisor host
+ *
+ * Copyright Microsoft Corp. 2020-2021
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#pragma once
+
+int virHostValidateCh(void);
diff --git a/tools/virt-host-validate.c b/tools/virt-host-validate.c
index 806d61bc8e..4dae97910a 100644
--- a/tools/virt-host-validate.c
+++ b/tools/virt-host-validate.c
@@ -40,6 +40,9 @@
 #if WITH_BHYVE
 # include "virt-host-validate-bhyve.h"
 #endif
+#if WITH_CH
+# include "virt-host-validate-ch.h"
+#endif
 
 static void
 show_help(FILE *out, const char *argv0)
@@ -53,6 +56,7 @@ show_help(FILE *out, const char *argv0)
               "   - qemu\n"
               "   - lxc\n"
               "   - bhyve\n"
+              "   - ch\n"
               "\n"
               " Options:\n"
               "   -h, --help     Display command line help\n"
@@ -146,6 +150,14 @@ main(int argc, char **argv)
     }
 #endif
 
+#if WITH_CH
+    if (!hvname || STREQ(hvname, "ch")) {
+        usedHvname = true;
+        if (virHostValidateCh() < 0)
+            ret = EXIT_FAILURE;
+    }
+#endif
+
     if (hvname && !usedHvname) {
         fprintf(stderr, _("%s: unsupported hypervisor name %s\n"),
                 argv[0], hvname);
-- 
2.27.0


Re: [PATCH v1] tools: add virt-host-validate-ch for ch driver
Posted by Michal Prívozník 2 years, 6 months ago
On 10/7/21 11:49 PM, Praveen K Paladugu wrote:
> Signed-off-by: Wei-Chen Chen <weicche@microsoft.com>
> Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
> ---
>  po/POTFILES.in                |  1 +
>  tools/meson.build             |  5 +++
>  tools/virt-host-validate-ch.c | 85 +++++++++++++++++++++++++++++++++++
>  tools/virt-host-validate-ch.h | 24 ++++++++++
>  tools/virt-host-validate.c    | 12 +++++
>  5 files changed, 127 insertions(+)
>  create mode 100644 tools/virt-host-validate-ch.c
>  create mode 100644 tools/virt-host-validate-ch.h
> 
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index c200d7452a..b554cf08ca 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -369,6 +369,7 @@
>  @SRCDIR@tools/virsh.h
>  @SRCDIR@tools/virt-admin.c
>  @SRCDIR@tools/virt-host-validate-bhyve.c
> +@SRCDIR@tools/virt-host-validate-ch.c
>  @SRCDIR@tools/virt-host-validate-common.c
>  @SRCDIR@tools/virt-host-validate-lxc.c
>  @SRCDIR@tools/virt-host-validate-qemu.c
> diff --git a/tools/meson.build b/tools/meson.build
> index 2acf7b0aaf..bf0eab8b6b 100644
> --- a/tools/meson.build
> +++ b/tools/meson.build
> @@ -58,6 +58,11 @@ if conf.has('WITH_HOST_VALIDATE')
>        'virt-host-validate-bhyve.c',
>      ]
>    endif
> +  if conf.has('WITH_CH')
> +    virt_host_validate_sources += [
> +      'virt-host-validate-ch.c',
> +    ]
> +  endif
>  
>    executable(
>      'virt-host-validate',
> diff --git a/tools/virt-host-validate-ch.c b/tools/virt-host-validate-ch.c
> new file mode 100644
> index 0000000000..a6d8a01d1b
> --- /dev/null
> +++ b/tools/virt-host-validate-ch.c
> @@ -0,0 +1,85 @@
> +/*
> + * Copyright Microsoft Corp. 2020-2021
> + *
> + * virt-host-validate-ch.c: Sanity check a CH hypervisor host

We usually put this line first and Copyright after that.

> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <config.h>
> +
> +#include "virarch.h"
> +#include "virbitmap.h"
> +#include "virt-host-validate-ch.h"
> +#include "virt-host-validate-common.h"
> +
> +int virHostValidateCh(void)
> +{
> +  int ret = 0;
> +  virBitmap *flags;
> +  bool hasHwVirt = false;
> +  bool hasVirtFlag = false;
> +  virArch arch = virArchFromHost();
> +  const char *kvmhint =
> +      _("Check that CPU and firmware supports virtualization "
> +        "and kvm module is loaded");
> +

This entire function is not formatted according to our guidelines.

https://libvirt.org/coding-style.html

> +  if (!(flags = virHostValidateGetCPUFlags()))
> +    return -1;
> +
> +  // Cloud-Hypervisor only supports x86_64 and aarch64

We are a bit old school and like C89 style of comments.

These are all small nits that I can fix before pushing.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Congratulations on your first libvirt contribution!

Michal

Re: [PATCH v1] tools: add virt-host-validate-ch for ch driver
Posted by Praveen K Paladugu 2 years, 6 months ago

On 10/12/2021 10:32 AM, Michal Prívozník wrote:
> On 10/7/21 11:49 PM, Praveen K Paladugu wrote:
>> Signed-off-by: Wei-Chen Chen <weicche@microsoft.com>
>> Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
>> ---
>>   po/POTFILES.in                |  1 +
>>   tools/meson.build             |  5 +++
>>   tools/virt-host-validate-ch.c | 85 +++++++++++++++++++++++++++++++++++
>>   tools/virt-host-validate-ch.h | 24 ++++++++++
>>   tools/virt-host-validate.c    | 12 +++++
>>   5 files changed, 127 insertions(+)
>>   create mode 100644 tools/virt-host-validate-ch.c
>>   create mode 100644 tools/virt-host-validate-ch.h
>>
>> diff --git a/po/POTFILES.in b/po/POTFILES.in
>> index c200d7452a..b554cf08ca 100644
>> --- a/po/POTFILES.in
>> +++ b/po/POTFILES.in
>> @@ -369,6 +369,7 @@
>>   @SRCDIR@tools/virsh.h
>>   @SRCDIR@tools/virt-admin.c
>>   @SRCDIR@tools/virt-host-validate-bhyve.c
>> +@SRCDIR@tools/virt-host-validate-ch.c
>>   @SRCDIR@tools/virt-host-validate-common.c
>>   @SRCDIR@tools/virt-host-validate-lxc.c
>>   @SRCDIR@tools/virt-host-validate-qemu.c
>> diff --git a/tools/meson.build b/tools/meson.build
>> index 2acf7b0aaf..bf0eab8b6b 100644
>> --- a/tools/meson.build
>> +++ b/tools/meson.build
>> @@ -58,6 +58,11 @@ if conf.has('WITH_HOST_VALIDATE')
>>         'virt-host-validate-bhyve.c',
>>       ]
>>     endif
>> +  if conf.has('WITH_CH')
>> +    virt_host_validate_sources += [
>> +      'virt-host-validate-ch.c',
>> +    ]
>> +  endif
>>   
>>     executable(
>>       'virt-host-validate',
>> diff --git a/tools/virt-host-validate-ch.c b/tools/virt-host-validate-ch.c
>> new file mode 100644
>> index 0000000000..a6d8a01d1b
>> --- /dev/null
>> +++ b/tools/virt-host-validate-ch.c
>> @@ -0,0 +1,85 @@
>> +/*
>> + * Copyright Microsoft Corp. 2020-2021
>> + *
>> + * virt-host-validate-ch.c: Sanity check a CH hypervisor host
> 
> We usually put this line first and Copyright after that.
> 
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library.  If not, see
>> + * <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <config.h>
>> +
>> +#include "virarch.h"
>> +#include "virbitmap.h"
>> +#include "virt-host-validate-ch.h"
>> +#include "virt-host-validate-common.h"
>> +
>> +int virHostValidateCh(void)
>> +{
>> +  int ret = 0;
>> +  virBitmap *flags;
>> +  bool hasHwVirt = false;
>> +  bool hasVirtFlag = false;
>> +  virArch arch = virArchFromHost();
>> +  const char *kvmhint =
>> +      _("Check that CPU and firmware supports virtualization "
>> +        "and kvm module is loaded");
>> +
> 
> This entire function is not formatted according to our guidelines.
> 
> https://libvirt.org/coding-style.html
> 
Will pay attention to coding style for future patches.

>> +  if (!(flags = virHostValidateGetCPUFlags()))
>> +    return -1;
>> +
>> +  // Cloud-Hypervisor only supports x86_64 and aarch64
> 
> We are a bit old school and like C89 style of comments.
> 
> These are all small nits that I can fix before pushing.
> 
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> 
> Congratulations on your first libvirt contribution!

Thank you!!
> 
> Michal
> 

-- 
Regards,
Praveen K Paladugu