Re: [PATCH v8 9/9] qtest/hyperv: Introduce a simple hyper-v test

[Igor Mammedov]

On Fri, 16 Jul 2021 14:12:06 +0200
Vitaly Kuznetsov <vkuznets@redhat.com> wrote:

> Igor Mammedov <imammedo@redhat.com> writes:
> 
> > On Thu, 8 Jul 2021 17:02:22 -0400
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >  
> >> On Tue, Jun 08, 2021 at 02:08:17PM +0200, Vitaly Kuznetsov wrote:  
> >> > For the beginning, just test 'hv-passthrough' and a couple of custom
> >> > Hyper-V  enlightenments configurations through QMP. Later, it would
> >> > be great to complement this by checking CPUID values from within the
> >> > guest.
> >> > 
> >> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>    
> >> [...]  
> >> > +static bool kvm_has_sys_hyperv_cpuid(void)
> >> > +{
> >> > +    int fd = open("/dev/kvm", O_RDWR);
> >> > +    int ret;
> >> > +
> >> > +    g_assert(fd > 0);    
> >>   
> 
> g_assert() was an overkill, just 'return false' would do.
> 
> >> This crashes when /dev/kvm doesn't exist.  See:
> >> https://gitlab.com/ehabkost/qemu/-/jobs/1404084459  
> >
> > maybe reuse qtest_has_accel()
> >  https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg06864.html
> >
> > instead of op encoding it.  
> 
> The purpose of this function is to check if KVM_CAP_SYS_HYPERV_CPUID is
> supported by KVM. It is certainly unsupported when KVM is not present
> :-) but an ioctl() is needed when it is.
> 
> We already have a similar check in tests/qtest/migration-test.c where we
> test for KVM_CAP_DIRTY_LOG_RING, maybe we can create a library function
> but we don't seem to have any KVM-specific stuff in qtest at this moment

qtest_has_accel() is a such library function
in the same series see 
https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg06878.html
which replaces custom kvm probing in tests/qtest/migration-test.c

> ...
> 
> >> I'm removing it from the queue.  
> 
> I'll fix g_assert() and send as a separate patch if it's fine.
>