add virt-host-validate-ch for cloud-hypervisor #13

zarcen · 2020-09-14T18:03:00Z

Fixes #12
Add initial support of virt-host-validate for cloud-hypervisor

CH runs on top of KVM. So, it does the same check for KVM as QEMU
Check minimum cloud-hypervisor version to meet >= 0.10.0

Output example:

$ virt-host-validate ch
    CH: Checking for hardware virtualization                                 : PASS
    CH: Checking if device /dev/kvm exists                                   : PASS
    CH: Checking if device /dev/kvm is accessible                            : PASS

sameo · 2020-09-16T07:29:28Z

tools/virt-host-validate-ch.c

+	}
+
+	char verResult[256];
+	virHostMsgCheck("CH", "Cloud-Hypvervisor >= 0.9.0");


0.10.0 will be released on 09/17, so you may want to check for 0.10.0 already.

Changed to check for 0.10.0

sameo · 2020-09-16T07:30:27Z

tools/virt-host-validate-ch.c

+    while(ptr != NULL)
+	{
+        last_tok = ptr;
+		ptr = strtok(NULL, " ");


There seems to be a few coding style issues with that PR. Have you run ninja -C build test against it?

Addressed all test defects in ninja -C build test

bryteise · 2020-09-18T12:33:05Z

tools/virt-host-validate-ch.c

+		return -1;
+
+	switch ((int)arch) {
+		case VIR_ARCH_I686:


I'm not sure about this arch check given the supported arch list. It probably should just be checks on VIR_ARCH_X86_64 and VIR_ARCH_AARCH64.

Ah I see the style issues also fixed up most of the arch checks sorry. Still I think the I686 test should be removed.

Removed I686.

bryteise · 2020-09-18T12:37:06Z

tools/virt-host-validate-ch.c

+	{
+		return -1;
+	}
+}


Is there a reason this is being done differently than chExtractVersionInto? I did get a little feedback for simplifying that even in the RFC review here.

After reviewing it again, I'd decide to remove this check as it is already covered by driver already. Instead, this place should only do prerequisite environment check.

bryteise · 2020-09-25T13:14:27Z

LGTM

zarcen closed this Sep 14, 2020

zarcen reopened this Sep 14, 2020

zarcen force-pushed the ch_validate-host branch from aed1b69 to 060d7bd Compare September 14, 2020 18:48

add virt-host-validate-ch for cloud-hypervisor

0b58073

zarcen force-pushed the ch_validate-host branch from 060d7bd to 0b58073 Compare September 14, 2020 18:53

sameo requested a review from bryteise September 15, 2020 08:09

sameo reviewed Sep 16, 2020

View reviewed changes

fix style issue reported in ninja build test

e4e7bbf

zarcen force-pushed the ch_validate-host branch from 66b4307 to e4e7bbf Compare September 18, 2020 06:25

bryteise reviewed Sep 18, 2020

View reviewed changes

remove redundant check for version

7290a63

zarcen force-pushed the ch_validate-host branch from caecc49 to 7290a63 Compare September 23, 2020 11:21

sameo merged commit faccded into cloud-hypervisor:ch Sep 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add virt-host-validate-ch for cloud-hypervisor #13

add virt-host-validate-ch for cloud-hypervisor #13

zarcen commented Sep 14, 2020 •

edited

Loading

sameo Sep 16, 2020

zarcen Sep 18, 2020

sameo Sep 16, 2020

zarcen Sep 18, 2020

bryteise Sep 18, 2020

bryteise Sep 18, 2020

zarcen Sep 23, 2020

bryteise Sep 18, 2020

zarcen Sep 23, 2020 •

edited

Loading

bryteise commented Sep 25, 2020

add virt-host-validate-ch for cloud-hypervisor #13

add virt-host-validate-ch for cloud-hypervisor #13

Conversation

zarcen commented Sep 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zarcen Sep 23, 2020 • edited Loading

Choose a reason for hiding this comment

bryteise commented Sep 25, 2020

zarcen commented Sep 14, 2020 •

edited

Loading

zarcen Sep 23, 2020 •

edited

Loading