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

On 10/7/21 11:49 PM, Praveen K Paladugu wrote: > Signed-off-by: Wei-Chen Chen <weicche(a)microsoft.com&gt; > Signed-off-by: Praveen K Paladugu <prapal(a)linux.microsoft.com&gt; > --- > 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(a)tools/virsh.h > @SRCDIR(a)tools/virt-admin.c > @SRCDIR(a)tools/virt-host-validate-bhyve.c > +@SRCDIR(a)tools/virt-host-validate-ch.c > @SRCDIR(a)tools/virt-host-validate-common.c > @SRCDIR(a)tools/virt-host-validate-lxc.c > @SRCDIR(a)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(a)redhat.com&gt; Congratulations on your first libvirt contribution!

Michal