Re: [PATCH v2] ch: Enable hyperv hypervisor

On 2/20/24 23:06, Praveen K Paladugu wrote: > From: Praveen K Paladugu <prapal(a)linux.microsoft.com&gt; > > Cloud-Hypervisor is capable of running VMs with kvm or mshv as the > hypervisor on Linux Host. Guest to hypevisor ABI with mshv hypervisor is > the same as in the case of VIR_DOMAIN_VIRT_HYPERV. So, VIR_DOMAIN_VIRT_HYPERV > type will be reused to represent the config with Linux Host and mshv as the > hypervisor. > > While initializing ch driver, check if either of /dev/kvm or /dev/mshv > device is present on the host. Before starting ch domains, check if the > requested hypervisor device is present on the host. > > Users can specify hypervisor in ch guests's domain definitions like > below: > > <domain type='kvm'> > > _or_ > > <domain type='hyperv'> > > Signed-off-by: Praveen K Paladugu <prapal(a)linux.microsoft.com&gt; > Signed-off-by: Praveen K Paladugu <praveenkpaladugu(a)gmail.com&gt; > --- > src/ch/ch_conf.c | 2 ++ > src/ch/ch_driver.c | 7 +++++++ > src/ch/ch_process.c | 35 +++++++++++++++++++++++++++++++++++ > 3 files changed, 44 insertions(+) > > diff --git a/src/ch/ch_conf.c b/src/ch/ch_conf.c > index f421af5121..1911ae8f8b 100644 > --- a/src/ch/ch_conf.c > +++ b/src/ch/ch_conf.c > @@ -69,6 +69,8 @@ virCaps *virCHDriverCapsInit(void) > > virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_KVM, > NULL, NULL, 0, NULL); > + virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_HYPERV, > + NULL, NULL, 0, NULL); 1: This sets support for both virtTypes unconditionally even though only one might be supported. Problem with this approach is: I, as an user, check for supported virtTypes (e.g. via 'virsh capabilities') find hyperv supported only to get an error when trying to start such domain. > return g_steal_pointer(&caps); > } > > diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c > index 96de5044ac..d6294c76ee 100644 > --- a/src/ch/ch_driver.c > +++ b/src/ch/ch_driver.c > @@ -32,6 +32,7 @@ > #include "viraccessapicheck.h" > #include "virchrdev.h" > #include "virerror.h" > +#include "virfile.h" > #include "virlog.h" > #include "virobject.h" > #include "virtypedparam.h" > @@ -876,6 +877,12 @@ static int chStateInitialize(bool privileged, > return -1; > } > > + if (!(virFileExists("/dev/kvm") || virFileExists("/dev/mshv"))) { > + virReportError(VIR_ERR_DEVICE_MISSING, "%s", VIR_ERR_DEVICE_MISSING code should be used for cases where a device is looked up in domain config but it's not found (e.g. on hotunplug). Though it's used in one other (unrelated?) case too: virMediatedDeviceNew() - which is transitively called from domain handling code. But more importantly, this check needs to go to caps init [1] and here we should merely check whether caps has at least one of the virtTypes set (e.g. via virCapabilitiesDomainSupported()).

> + _("/dev/kvm and /dev/mshv. ch driver failed to initialize.")); > + return VIR_DRV_STATE_INIT_ERROR; > + } > + > ch_driver = g_new0(virCHDriver, 1);