Re: [libvirt] [PATCH 08/22] Introduce virConnectCompareHypervisorCPU public API

On 05/16/2018 04:39 AM, Jiri Denemark wrote: > This new API compares the given CPU description with the CPU the > specified hypervisor is able to provide on the host. It is a more useful > version of virConnectCompareCPU, which compares the CPU definition with > the host CPU without considering any specific hypervisor and its > abilities. > > Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; > --- > include/libvirt/libvirt-host.h | 7 ++++ > src/driver-hypervisor.h | 10 +++++ > src/libvirt-host.c | 72 +++++++++++++++++++++++++++++++++- > src/libvirt_public.syms | 5 +++ > 4 files changed, 93 insertions(+), 1 deletion(-) > > diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h > index 07b5d15943..e2054baebc 100644 > --- a/include/libvirt/libvirt-host.h > +++ b/include/libvirt/libvirt-host.h > @@ -640,6 +640,13 @@ typedef enum { > int virConnectCompareCPU(virConnectPtr conn, > const char *xmlDesc, > unsigned int flags); > +int virConnectCompareHypervisorCPU(virConnectPtr conn, > + const char *emulator, > + const char *arch, > + const char *machine, > + const char *virttype, > + const char *xmlCPU, > + unsigned int flags); > > int virConnectGetCPUModelNames(virConnectPtr conn, > const char *arch, > diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h > index e71a72a441..d64de2d54c 100644 > --- a/src/driver-hypervisor.h > +++ b/src/driver-hypervisor.h > @@ -673,6 +673,15 @@ typedef int > const char *cpu, > unsigned int flags); > > +typedef int > +(*virDrvConnectCompareHypervisorCPU)(virConnectPtr conn, > + const char *emulator, > + const char *arch, > + const char *machine, > + const char *virttype, > + const char *xmlCPU, > + unsigned int flags); > + > typedef char * > (*virDrvConnectBaselineCPU)(virConnectPtr conn, > const char **xmlCPUs, > @@ -1532,6 +1541,7 @@ struct _virHypervisorDriver { > virDrvDomainSetVcpu domainSetVcpu; > virDrvDomainSetBlockThreshold domainSetBlockThreshold; > virDrvDomainSetLifecycleAction domainSetLifecycleAction; > + virDrvConnectCompareHypervisorCPU connectCompareHypervisorCPU; > }; > > > diff --git a/src/libvirt-host.c b/src/libvirt-host.c > index ed689b9ec2..17cf183499 100644 > --- a/src/libvirt-host.c > +++ b/src/libvirt-host.c > @@ -954,7 +954,11 @@ virConnectIsSecure(virConnectPtr conn) > * @xmlDesc: XML describing the CPU to compare with host CPU > * @flags: bitwise-OR of virConnectCompareCPUFlags > * > - * Compares the given CPU description with the host CPU > + * Compares the given CPU description with the host CPU. > + * > + * See vitConnectCompareHypervisorCPU() if you want to consider hypervisor > + * abilities and compare the CPU to the CPU which a hypervisor is able to > + * provide on the host. > * > * Returns comparison result according to enum virCPUCompareResult. If > * VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE is used and @xmlDesc CPU is > @@ -992,6 +996,72 @@ virConnectCompareCPU(virConnectPtr conn, > } > > > +/** > + * virConnectCompareHypervisorCPU: > + * @conn: pointer to the hypervisor connection > + * @emulator: path to the emulator binary > + * @arch: domain architecture > + * @machine: machine type > + * @virttype: virtualization type > + * @xmlCPU: XML describing the CPU to be compared > + * @flags: bitwise-OR of virConnectCompareCPUFlags > + * > + * Compares the given CPU description with the CPU the specified hypervisor is > + * able to provide on the host. Any of @emulator, @arch, @machine, and > + * @virttype parameters may be NULL; libvirt will choose sensible defaults > + * tailored to the host and its current configuration. > + * > + * This is different from virConnectCompareCPU() which compares the CPU > + * definition with the host CPU without considering any specific hypervisor and > + * its abilities. > + * > + * Returns comparison result according to enum virCPUCompareResult. If > + * VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE is used and @xmlCPU is > + * incompatible with the CPU the specified hypervisor is able to provide on the > + * host, this function will return VIR_CPU_COMPARE_ERROR (instead of > + * VIR_CPU_COMPARE_INCOMPATIBLE) and the error will use the > + * VIR_ERR_CPU_INCOMPATIBLE code with a message providing more details about > + * the incompatibility. > + */ > +int > +virConnectCompareHypervisorCPU(virConnectPtr conn, > + const char *emulator, > + const char *arch, > + const char *machine, > + const char *virttype, > + const char *xmlCPU, > + unsigned int flags) > +{ > + VIR_DEBUG("conn=%p, emulator=%s, arch=%s, machine=%s, " > + "virttype=%s, xmlCPU=%s, flags=0x%x", > + conn, NULLSTR(emulator), NULLSTR(arch), NULLSTR(machine), > + NULLSTR(virttype), NULLSTR(xmlCPU), flags); > + > + virResetLastError(); > + > + virCheckConnectReturn(conn, VIR_CPU_COMPARE_ERROR); > + virCheckNonNullArgGoto(xmlCPU, error); > + > + if (conn->driver->connectCompareHypervisorCPU) { > + int ret; > + > + ret = conn->driver->connectCompareHypervisorCPU(conn, emulator, arch, > + machine, virttype, > + xmlCPU, flags); > + if (ret == VIR_CPU_COMPARE_ERROR) > + goto error; Admittedly I did not look too closely, but will the compareHypervisorCPU functions actually return "VIR_CPU_COMPARE_ERROR" on error? If so, wouldn't it be more sensible to return a "VIR_HYPERVISOR_CPU_COMPARE_ERROR" instead?