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

[Praveen K Paladugu]

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 at microsoft.com>
>> Signed-off-by: Praveen K Paladugu <prapal at 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 at tools/virsh.h
>>   @SRCDIR at tools/virt-admin.c
>>   @SRCDIR at tools/virt-host-validate-bhyve.c
>> + at SRCDIR@tools/virt-host-validate-ch.c
>>   @SRCDIR at tools/virt-host-validate-common.c
>>   @SRCDIR at tools/virt-host-validate-lxc.c
>>   @SRCDIR at 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 at redhat.com>
> 
> Congratulations on your first libvirt contribution!

Thank you!!
> 
> Michal
> 

-- 
Regards,
Praveen K Paladugu