[libvirt PATCH 0/9] [RFC] Dynamic CPU models
libvirt and qemu cpu models are out of sync. libvirt cpu models are considered static and never changing, whereas qemu cpu models have changed over time. This leads to problems and confusion for users, as the cpu models they specify may have different features than what they expect. This series introduces two new APIs to enumerate and retrieve cpu models dynamically from the hypervisor at runtime, allowing the user to make use of cpu models from newer hypervisor versions even if libvirt is not aware of them. These two new functions are intended as building blocks onto which higher-level functionality can be build, e.g. the ability to specify a hypervisor cpu model directly in a domain xml. With these two functions alone this has to be done manually for now, i.e. enumerate the available cpu models, retrieve the cpu description and manually insert it into the domain xml accordingly: $ virsh hypervisor-cpu-models --arch x86_64 Name Alias ---------------------------------------------------- max host base qemu64-v1 qemu64 qemu64-v1 qemu32-v1 qemu32 qemu32-v1 phenom-v1 phenom phenom-v1 (...) $ virsh hypervisor-cpu-definition --machine pc --arch x86_64 qemu64 <cpu> <model>qemu64</model> <feature name='cmov'/> <feature name='mmx'/> <feature name='xd'/> <feature name='x-intel-pt-auto-level'/> <feature name='kvm_asyncpf'/> <feature name='kvm-asyncpf'/> <feature name='legacy-cache'/> <feature name='vmware-cpuid-freq'/> <feature name='mce'/> <feature name='mca'/> <feature name='msr'/> <feature name='fxsr'/> <feature name='cpuid-0xb'/> <feature name='kvm_pv_eoi'/> <feature name='pni'/> <feature name='x2apic'/> <feature name='i64'/> <feature name='pae'/> <feature name='pat'/> <feature name='sse'/> <feature name='kvm_nopiodelay'/> <feature name='kvm-nopiodelay'/> <feature name='kvmclock-stable-bit'/> <feature name='hypervisor'/> <feature name='syscall'/> <feature name='x-migrate-smi-count'/> <feature name='full-cpuid-auto-level'/> <feature name='sse3'/> <feature name='sse2'/> <feature name='kvm-pv-eoi'/> <feature name='cx8'/> <feature name='pge'/> <feature name='fill-mtrr-mask'/> <feature name='cx16'/> <feature name='de'/> <feature name='clflush'/> <feature name='tsc'/> <feature name='fpu'/> <feature name='check'/> <feature name='apic'/> <feature name='kvm-steal-time'/> <feature name='kvm_steal_time'/> <feature name='kvmclock'/> <feature name='l3-cache'/> <feature name='nx'/> <feature name='tcg-cpuid'/> <feature name='lm'/> <feature name='pse'/> <feature name='sep'/> <feature name='kvm'/> <feature name='lahf-lm'/> <feature name='lahf_lm'/> <feature name='mtrr'/> <feature name='pse36'/> </cpu> The returned cpu description is currently completely unfiltered, as can be seen by the duplicate entries differing only in "-" vs. "_" usage, inclusion of experimental feature flags and generally, flags that are not recognized by libvirt. One possibility to adress this would be to extend virQEMUCapsCPUFeatureTranslate. Before I continue with this, I would love to hear other people's thoughts, comments and potential use cases. Cheers, Tim Tim Wiederhake (9): qemuMonitorCPUDefInfo: Add alias libvirt: introduce virConnectGetHypervisorCPUModelNames public API remote: Add RPC support for the virConnectGetHypervisorCPUModelNames API qemu: implement virConnectGetHypervisorCPUModelNames API tools: Report hypervisor cpu model names libvirt: introduce virConnectGetHypervisorCPUModelDefinition public API remote: Add support for the virConnectGetHypervisorCPUModelDefinition API qemu: Implement virConnectGetHypervisorCPUModelDefinition API tools: Report hypervisor cpu model definitions docs/manpages/virsh.rst | 27 ++++++ include/libvirt/libvirt-host.h | 13 +++ src/driver-hypervisor.h | 17 ++++ src/libvirt-host.c | 109 +++++++++++++++++++++++ src/libvirt_public.syms | 2 + src/qemu/qemu_driver.c | 105 ++++++++++++++++++++++ src/qemu/qemu_monitor.c | 2 + src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 3 + src/remote/remote_daemon_dispatch.c | 76 ++++++++++++++++ src/remote/remote_driver.c | 100 +++++++++++++++++++++ src/remote/remote_protocol.x | 37 +++++++- src/remote_protocol-structs | 27 ++++++ tools/virsh-host.c | 132 ++++++++++++++++++++++++++++ 14 files changed, 650 insertions(+), 1 deletion(-) -- 2.31.1
Preserve the "alias-of" information of the qemu response that indicates that the cpu model is merely an alternate name for another cpu model. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_monitor.c | 2 ++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 3 +++ 3 files changed, 6 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index fda5d2f368..ccea577ebc 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3363,6 +3363,7 @@ qemuMonitorCPUDefsFree(qemuMonitorCPUDefs *defs) for (i = 0; i < defs->ncpus; i++) { g_strfreev(defs->cpus[i].blockers); g_free(defs->cpus[i].name); + g_free(defs->cpus[i].alias); g_free(defs->cpus[i].type); } @@ -3400,6 +3401,7 @@ qemuMonitorCPUDefsCopy(qemuMonitorCPUDefs *src) cpuDst->usable = cpuSrc->usable; cpuDst->name = g_strdup(cpuSrc->name); + cpuDst->alias = g_strdup(cpuSrc->alias); cpuDst->type = g_strdup(cpuSrc->type); cpuDst->blockers = g_strdupv(cpuSrc->blockers); cpuDst->deprecated = cpuSrc->deprecated; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 95267ec6c7..6cb4ff1e22 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1150,6 +1150,7 @@ typedef struct _qemuMonitorCPUDefInfo qemuMonitorCPUDefInfo; struct _qemuMonitorCPUDefInfo { virDomainCapsCPUUsable usable; char *name; + char *alias; char *type; char **blockers; /* NULL-terminated string list */ bool deprecated; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3aad2ab212..4ffd3bbed7 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5274,6 +5274,9 @@ qemuMonitorJSONGetCPUDefinitions(qemuMonitor *mon, cpu->name = g_strdup(tmp); + if ((tmp = virJSONValueObjectGetString(child, "alias-of")) && *tmp) + cpu->alias = g_strdup(tmp); + if ((tmp = virJSONValueObjectGetString(child, "typename")) && *tmp) cpu->type = g_strdup(tmp); -- 2.31.1
On Tue, Jun 28, 2022 at 06:09:38PM +0200, Tim Wiederhake wrote:
Preserve the "alias-of" information of the qemu response that indicates that the cpu model is merely an alternate name for another cpu model.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_monitor.c | 2 ++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 3 +++ 3 files changed, 6 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Create a function to query the hypervisor for its list of known CPU model names. This is different from virConnectGetCPUModelNames, as this new function will determine the list of CPU models (and alias names) at runtime. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- include/libvirt/libvirt-host.h | 6 ++++ src/driver-hypervisor.h | 8 +++++ src/libvirt-host.c | 55 ++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 70 insertions(+) diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index 3112f2b676..5aaa001adb 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -962,6 +962,12 @@ int virConnectGetCPUModelNames(virConnectPtr conn, char ***models, unsigned int flags); +int virConnectGetHypervisorCPUModelNames(virConnectPtr conn, + const char *arch, + char ***names, + char ***aliases, + unsigned int flags); + /** * virConnectBaselineCPUFlags: * diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 016d5cec7c..c81e5d4c75 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -732,6 +732,13 @@ typedef int char ***models, unsigned int flags); +typedef int +(*virDrvConnectGetHypervisorCPUModelNames)(virConnectPtr conn, + const char *archName, + char ***names, + char ***aliases, + unsigned int flags); + typedef int (*virDrvDomainGetJobInfo)(virDomainPtr domain, virDomainJobInfoPtr info); @@ -1712,4 +1719,5 @@ struct _virHypervisorDriver { virDrvDomainAuthorizedSSHKeysSet domainAuthorizedSSHKeysSet; virDrvDomainGetMessages domainGetMessages; virDrvDomainStartDirtyRateCalc domainStartDirtyRateCalc; + virDrvConnectGetHypervisorCPUModelNames connectGetHypervisorCPUModelNames; }; diff --git a/src/libvirt-host.c b/src/libvirt-host.c index 2ee6370bce..6e734628c1 100644 --- a/src/libvirt-host.c +++ b/src/libvirt-host.c @@ -1234,6 +1234,61 @@ virConnectGetCPUModelNames(virConnectPtr conn, const char *arch, char ***models, } +/** + * virConnectGetHypervisorCPUModelNames: + * + * @conn: virConnect connection + * @arch: Architecture + * @names: Pointer to a variable to store the NULL-terminated array of the CPU + * models supported by the hypervisor for the specified architecture. + * Each element and the array itself must be freed by the caller. + * @aliases: Pointer to a variable to store the NULL-terminated array of alias + * names for CPU model names. Each element and the array itself must + * be freed by the caller. + * @flags: extra flags; not used yet, so callers should always pass 0. + * + * Get the list of CPU models supported by the hypervisor for a specific + * architecture. + * + * If @aliases[x] is not an empty string, @names[x] is an alias for that CPU + * model. + * + * Returns -1 on error, number of elements in @models on success. + * + * Since: 8.5.0 + */ +int +virConnectGetHypervisorCPUModelNames(virConnectPtr conn, + const char *arch, + char ***names, + char ***aliases, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, arch=%s, flags=0x%x", conn, NULLSTR(arch), flags); + virResetLastError(); + + virCheckConnectReturn(conn, -1); + virCheckNonNullArgGoto(arch, error); + + if (conn->driver->connectGetHypervisorCPUModelNames) { + int ret; + + ret = conn->driver->connectGetHypervisorCPUModelNames(conn, arch, names, + aliases, flags); + if (ret < 0) + goto error; + + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(conn); + return -1; +} + + /** * virConnectBaselineCPU: * diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 297a2c436a..c6a8e898ae 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -924,6 +924,7 @@ LIBVIRT_8.4.0 { LIBVIRT_8.5.0 { global: + virConnectGetHypervisorCPUModelNames; virDomainAbortJobFlags; } LIBVIRT_8.4.0; -- 2.31.1
On Tue, Jun 28, 2022 at 06:09:39PM +0200, Tim Wiederhake wrote:
Create a function to query the hypervisor for its list of known CPU model names. This is different from virConnectGetCPUModelNames, as this new function will determine the list of CPU models (and alias names) at runtime.
This is duplicating what we already report in the per-domain level capabilities XML from virConnectGetDomainCapabilities. The original virConnectGetCPUModelNames pre-dated the addition of virConnectGetDomainCapabilities.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- include/libvirt/libvirt-host.h | 6 ++++ src/driver-hypervisor.h | 8 +++++ src/libvirt-host.c | 55 ++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 70 insertions(+)
diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index 3112f2b676..5aaa001adb 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -962,6 +962,12 @@ int virConnectGetCPUModelNames(virConnectPtr conn, char ***models, unsigned int flags);
+int virConnectGetHypervisorCPUModelNames(virConnectPtr conn, + const char *arch, + char ***names, + char ***aliases, + unsigned int flags);
For virConnectCompareHypervisorCPU, virConnectBaselineHypervisorCPU, and virConnectGetDomainCapabilities, we take more than just the 'arch' value: const char *emulator, const char *arch, const char *machine, const char *virttype, because we need all 4 of those pieces of information to accurately determine what qemu-system-XXXX binary we're going to select. If we do still want to add virConnectGetHypervisorCPUModelNames despite having the info on virConnectGetDomainCapabilities, we should take the full set of params. I guess adding the new API isn't too costly
+ /** * virConnectBaselineCPUFlags: * diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 016d5cec7c..c81e5d4c75 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -732,6 +732,13 @@ typedef int char ***models, unsigned int flags);
+typedef int +(*virDrvConnectGetHypervisorCPUModelNames)(virConnectPtr conn, + const char *archName, + char ***names, + char ***aliases, + unsigned int flags); + typedef int (*virDrvDomainGetJobInfo)(virDomainPtr domain, virDomainJobInfoPtr info); @@ -1712,4 +1719,5 @@ struct _virHypervisorDriver { virDrvDomainAuthorizedSSHKeysSet domainAuthorizedSSHKeysSet; virDrvDomainGetMessages domainGetMessages; virDrvDomainStartDirtyRateCalc domainStartDirtyRateCalc; + virDrvConnectGetHypervisorCPUModelNames connectGetHypervisorCPUModelNames; }; diff --git a/src/libvirt-host.c b/src/libvirt-host.c index 2ee6370bce..6e734628c1 100644 --- a/src/libvirt-host.c +++ b/src/libvirt-host.c @@ -1234,6 +1234,61 @@ virConnectGetCPUModelNames(virConnectPtr conn, const char *arch, char ***models, }
+/** + * virConnectGetHypervisorCPUModelNames: + * + * @conn: virConnect connection + * @arch: Architecture + * @names: Pointer to a variable to store the NULL-terminated array of the CPU + * models supported by the hypervisor for the specified architecture. + * Each element and the array itself must be freed by the caller. + * @aliases: Pointer to a variable to store the NULL-terminated array of alias + * names for CPU model names. Each element and the array itself must + * be freed by the caller. + * @flags: extra flags; not used yet, so callers should always pass 0. + * + * Get the list of CPU models supported by the hypervisor for a specific + * architecture. + * + * If @aliases[x] is not an empty string, @names[x] is an alias for that CPU + * model. + * + * Returns -1 on error, number of elements in @models on success. + * + * Since: 8.5.0 + */ +int +virConnectGetHypervisorCPUModelNames(virConnectPtr conn, + const char *arch, + char ***names, + char ***aliases, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, arch=%s, flags=0x%x", conn, NULLSTR(arch), flags); + virResetLastError(); + + virCheckConnectReturn(conn, -1); + virCheckNonNullArgGoto(arch, error); + + if (conn->driver->connectGetHypervisorCPUModelNames) { + int ret; + + ret = conn->driver->connectGetHypervisorCPUModelNames(conn, arch, names, + aliases, flags); + if (ret < 0) + goto error; + + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(conn); + return -1; +} + + /** * virConnectBaselineCPU: * diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 297a2c436a..c6a8e898ae 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -924,6 +924,7 @@ LIBVIRT_8.4.0 {
LIBVIRT_8.5.0 { global: + virConnectGetHypervisorCPUModelNames; virDomainAbortJobFlags; } LIBVIRT_8.4.0;
-- 2.31.1
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, Jul 18, 2022 at 11:05:13AM +0100, Daniel P. Berrangé wrote:
On Tue, Jun 28, 2022 at 06:09:39PM +0200, Tim Wiederhake wrote:
Create a function to query the hypervisor for its list of known CPU model names. This is different from virConnectGetCPUModelNames, as this new function will determine the list of CPU models (and alias names) at runtime.
This is duplicating what we already report in the per-domain level capabilities XML from virConnectGetDomainCapabilities.
The original virConnectGetCPUModelNames pre-dated the addition of virConnectGetDomainCapabilities.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- include/libvirt/libvirt-host.h | 6 ++++ src/driver-hypervisor.h | 8 +++++ src/libvirt-host.c | 55 ++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 70 insertions(+)
diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index 3112f2b676..5aaa001adb 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -962,6 +962,12 @@ int virConnectGetCPUModelNames(virConnectPtr conn, char ***models, unsigned int flags);
+int virConnectGetHypervisorCPUModelNames(virConnectPtr conn, + const char *arch, + char ***names, + char ***aliases, + unsigned int flags);
For virConnectCompareHypervisorCPU, virConnectBaselineHypervisorCPU, and virConnectGetDomainCapabilities, we take more than just the 'arch' value:
const char *emulator, const char *arch, const char *machine, const char *virttype,
because we need all 4 of those pieces of information to accurately determine what qemu-system-XXXX binary we're going to select.
If we do still want to add virConnectGetHypervisorCPUModelNames despite having the info on virConnectGetDomainCapabilities, we should take the full set of params.
I guess adding the new API isn't too costly
Now I've read forward in the series, I see that there is no update to virConnectGetDomainCapabilities. I would expect that to be updated to reflect the alias information, as well as exposing the same list of CPUs as this new API. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Jun 28, 2022 at 06:09:39PM +0200, Tim Wiederhake wrote:
Create a function to query the hypervisor for its list of known CPU model names. This is different from virConnectGetCPUModelNames, as this new function will determine the list of CPU models (and alias names) at runtime.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- include/libvirt/libvirt-host.h | 6 ++++ src/driver-hypervisor.h | 8 +++++ src/libvirt-host.c | 55 ++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 70 insertions(+)
diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index 3112f2b676..5aaa001adb 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -962,6 +962,12 @@ int virConnectGetCPUModelNames(virConnectPtr conn, char ***models, unsigned int flags);
+int virConnectGetHypervisorCPUModelNames(virConnectPtr conn, + const char *arch, + char ***names, + char ***aliases, + unsigned int flags); + /** * virConnectBaselineCPUFlags: * diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 016d5cec7c..c81e5d4c75 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -732,6 +732,13 @@ typedef int char ***models, unsigned int flags);
+typedef int +(*virDrvConnectGetHypervisorCPUModelNames)(virConnectPtr conn, + const char *archName, + char ***names, + char ***aliases, + unsigned int flags); + typedef int (*virDrvDomainGetJobInfo)(virDomainPtr domain, virDomainJobInfoPtr info); @@ -1712,4 +1719,5 @@ struct _virHypervisorDriver { virDrvDomainAuthorizedSSHKeysSet domainAuthorizedSSHKeysSet; virDrvDomainGetMessages domainGetMessages; virDrvDomainStartDirtyRateCalc domainStartDirtyRateCalc; + virDrvConnectGetHypervisorCPUModelNames connectGetHypervisorCPUModelNames; }; diff --git a/src/libvirt-host.c b/src/libvirt-host.c index 2ee6370bce..6e734628c1 100644 --- a/src/libvirt-host.c +++ b/src/libvirt-host.c @@ -1234,6 +1234,61 @@ virConnectGetCPUModelNames(virConnectPtr conn, const char *arch, char ***models, }
+/** + * virConnectGetHypervisorCPUModelNames: + * + * @conn: virConnect connection + * @arch: Architecture + * @names: Pointer to a variable to store the NULL-terminated array of the CPU + * models supported by the hypervisor for the specified architecture. + * Each element and the array itself must be freed by the caller. + * @aliases: Pointer to a variable to store the NULL-terminated array of alias + * names for CPU model names. Each element and the array itself must + * be freed by the caller. + * @flags: extra flags; not used yet, so callers should always pass 0. + * + * Get the list of CPU models supported by the hypervisor for a specific + * architecture. + * + * If @aliases[x] is not an empty string, @names[x] is an alias for that CPU + * model.
Why use the empty string as a marker, as opposed to NULL which is the obvious "no data" marker ?
+ * + * Returns -1 on error, number of elements in @models on success. + * + * Since: 8.5.0 + */ +int
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Jun 28, 2022 at 06:09:39PM +0200, Tim Wiederhake wrote:
Create a function to query the hypervisor for its list of known CPU model names. This is different from virConnectGetCPUModelNames, as this new function will determine the list of CPU models (and alias names) at runtime.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- include/libvirt/libvirt-host.h | 6 ++++ src/driver-hypervisor.h | 8 +++++ src/libvirt-host.c | 55 ++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 70 insertions(+)
diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index 3112f2b676..5aaa001adb 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -962,6 +962,12 @@ int virConnectGetCPUModelNames(virConnectPtr conn, char ***models, unsigned int flags);
+int virConnectGetHypervisorCPUModelNames(virConnectPtr conn, + const char *arch, + char ***names, + char ***aliases, + unsigned int flags);
We have a notion of deprecation for CPU models. I wonder if we should include or omit deprecated CPU models by default, possibly determined based on a flag ? With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/remote/remote_daemon_dispatch.c | 44 +++++++++++++++++++++ src/remote/remote_driver.c | 59 +++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 19 +++++++++- src/remote_protocol-structs | 16 ++++++++ 4 files changed, 137 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index dc5790f077..9011977e18 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -5869,6 +5869,50 @@ remoteDispatchConnectGetCPUModelNames(virNetServer *server G_GNUC_UNUSED, } +static int +remoteDispatchConnectGetHypervisorCPUModelNames(virNetServer *server G_GNUC_UNUSED, + virNetServerClient *client, + virNetMessage *msg G_GNUC_UNUSED, + struct virNetMessageError *rerr, + remote_connect_get_hypervisor_cpu_model_names_args *args, + remote_connect_get_hypervisor_cpu_model_names_ret *ret) +{ + int len = 0; + int rv = -1; + g_auto(GStrv) names = NULL; + g_auto(GStrv) aliases = NULL; + virConnectPtr conn = remoteGetHypervisorConn(client); + + if (!conn) + goto cleanup; + + len = virConnectGetHypervisorCPUModelNames(conn, args->arch, &names, + &aliases, args->flags); + if (len < 0) + goto cleanup; + + if (len > REMOTE_CONNECT_CPU_MODELS_MAX) { + virReportError(VIR_ERR_RPC, + _("Too many CPU models '%d' for limit '%d'"), + len, REMOTE_CONNECT_CPU_MODELS_MAX); + goto cleanup; + } + + ret->names.names_val = g_steal_pointer(&names); + ret->names.names_len = len; + ret->aliases.aliases_val = g_steal_pointer(&aliases); + ret->aliases.aliases_len = len; + ret->ret = len; + + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + return rv; +} + + static int remoteDispatchDomainCreateXMLWithFiles(virNetServer *server G_GNUC_UNUSED, virNetServerClient *client, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 94566069f0..8dca23a7eb 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6393,6 +6393,64 @@ remoteConnectGetCPUModelNames(virConnectPtr conn, } +static int +remoteConnectGetHypervisorCPUModelNames(virConnectPtr conn, + const char *archName, + char ***names, + char ***aliases, + unsigned int flags) +{ + int rv = -1; + size_t i; + remote_connect_get_hypervisor_cpu_model_names_args args; + remote_connect_get_hypervisor_cpu_model_names_ret ret; + + struct private_data *priv = conn->privateData; + + remoteDriverLock(priv); + + args.arch = (char *) archName; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + if (call(conn, priv, 0, REMOTE_PROC_CONNECT_GET_HYPERVISOR_CPU_MODEL_NAMES, + (xdrproc_t) xdr_remote_connect_get_hypervisor_cpu_model_names_args, + (char *) &args, + (xdrproc_t) xdr_remote_connect_get_hypervisor_cpu_model_names_ret, + (char *) &ret) < 0) + goto done; + + /* Check the length of the returned list carefully. */ + if (ret.names.names_len > REMOTE_CONNECT_CPU_MODELS_MAX) { + virReportError(VIR_ERR_RPC, + _("Too many model names '%d' for limit '%d'"), + ret.names.names_len, + REMOTE_CONNECT_CPU_MODELS_MAX); + goto cleanup; + } + + *names = g_new0(char *, ret.names.names_len + 1); + for (i = 0; i < ret.names.names_len; i++) { + (*names)[i] = g_steal_pointer(&ret.names.names_val[i]); + } + + *aliases = g_new0(char *, ret.aliases.aliases_len + 1); + for (i = 0; i < ret.aliases.aliases_len; i++) { + (*aliases)[i] = g_steal_pointer(&ret.aliases.aliases_val[i]); + } + + rv = ret.ret; + + cleanup: + xdr_free((xdrproc_t) xdr_remote_connect_get_hypervisor_cpu_model_names_ret, + (char *) &ret); + + done: + remoteDriverUnlock(priv); + return rv; +} + + static int remoteDomainOpenGraphics(virDomainPtr dom, unsigned int idx, @@ -8652,6 +8710,7 @@ static virHypervisorDriver hypervisor_driver = { .domainGetMessages = remoteDomainGetMessages, /* 7.1.0 */ .domainStartDirtyRateCalc = remoteDomainStartDirtyRateCalc, /* 7.2.0 */ .domainSetLaunchSecurityState = remoteDomainSetLaunchSecurityState, /* 8.0.0 */ + .connectGetHypervisorCPUModelNames = remoteConnectGetHypervisorCPUModelNames, /* 8.5.0 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 79ffc63f03..a5c60399c7 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3303,6 +3303,17 @@ struct remote_connect_get_cpu_model_names_ret { int ret; }; +struct remote_connect_get_hypervisor_cpu_model_names_args { + remote_nonnull_string arch; + unsigned int flags; +}; + +struct remote_connect_get_hypervisor_cpu_model_names_ret { + remote_nonnull_string names<REMOTE_CONNECT_CPU_MODELS_MAX>; + remote_nonnull_string aliases<REMOTE_CONNECT_CPU_MODELS_MAX>; + int ret; +}; + struct remote_connect_network_event_register_any_args { int eventID; remote_network net; @@ -6959,5 +6970,11 @@ enum remote_procedure { * @generate: both * @acl: domain:write */ - REMOTE_PROC_DOMAIN_ABORT_JOB_FLAGS = 442 + REMOTE_PROC_DOMAIN_ABORT_JOB_FLAGS = 442, + + /** + * @generate: none + * @acl: connect:read + */ + REMOTE_PROC_CONNECT_GET_HYPERVISOR_CPU_MODEL_NAMES = 443 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index ca5222439d..c6afb92aad 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2671,6 +2671,21 @@ struct remote_connect_get_cpu_model_names_ret { } models; int ret; }; +struct remote_connect_get_hypervisor_cpu_model_names_args { + remote_nonnull_string arch; + u_int flags; +}; +struct remote_connect_get_hypervisor_cpu_model_names_ret { + struct { + u_int names_len; + remote_nonnull_string * names_val; + } names; + struct { + u_int aliases_len; + remote_nonnull_string * aliases_val; + } aliases; + int ret; +}; struct remote_connect_network_event_register_any_args { int eventID; remote_network net; @@ -3711,4 +3726,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SAVE_PARAMS = 440, REMOTE_PROC_DOMAIN_RESTORE_PARAMS = 441, REMOTE_PROC_DOMAIN_ABORT_JOB_FLAGS = 442, + REMOTE_PROC_CONNECT_GET_HYPERVISOR_CPU_MODEL_NAMES = 443, }; -- 2.31.1
On Tue, Jun 28, 2022 at 06:09:40PM +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/remote/remote_daemon_dispatch.c | 44 +++++++++++++++++++++ src/remote/remote_driver.c | 59 +++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 19 +++++++++- src/remote_protocol-structs | 16 ++++++++ 4 files changed, 137 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index dc5790f077..9011977e18 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -5869,6 +5869,50 @@ remoteDispatchConnectGetCPUModelNames(virNetServer *server G_GNUC_UNUSED, }
+static int +remoteDispatchConnectGetHypervisorCPUModelNames(virNetServer *server G_GNUC_UNUSED, + virNetServerClient *client, + virNetMessage *msg G_GNUC_UNUSED, + struct virNetMessageError *rerr, + remote_connect_get_hypervisor_cpu_model_names_args *args, + remote_connect_get_hypervisor_cpu_model_names_ret *ret) +{ + int len = 0; + int rv = -1; + g_auto(GStrv) names = NULL; + g_auto(GStrv) aliases = NULL; + virConnectPtr conn = remoteGetHypervisorConn(client); + + if (!conn) + goto cleanup; + + len = virConnectGetHypervisorCPUModelNames(conn, args->arch, &names, + &aliases, args->flags); + if (len < 0) + goto cleanup; + + if (len > REMOTE_CONNECT_CPU_MODELS_MAX) { + virReportError(VIR_ERR_RPC, + _("Too many CPU models '%d' for limit '%d'"), + len, REMOTE_CONNECT_CPU_MODELS_MAX); + goto cleanup; + } + + ret->names.names_val = g_steal_pointer(&names); + ret->names.names_len = len; + ret->aliases.aliases_val = g_steal_pointer(&aliases); + ret->aliases.aliases_len = len; + ret->ret = len;
'ret->ret' is not required, since both 'names' and 'aliases' already encode the length on the wire.
+ + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + return rv; +} + + static int remoteDispatchDomainCreateXMLWithFiles(virNetServer *server G_GNUC_UNUSED, virNetServerClient *client,
+static int +remoteConnectGetHypervisorCPUModelNames(virConnectPtr conn, + const char *archName, + char ***names, + char ***aliases, + unsigned int flags) +{ + int rv = -1; + size_t i; + remote_connect_get_hypervisor_cpu_model_names_args args; + remote_connect_get_hypervisor_cpu_model_names_ret ret; + + struct private_data *priv = conn->privateData; + + remoteDriverLock(priv); + + args.arch = (char *) archName; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + if (call(conn, priv, 0, REMOTE_PROC_CONNECT_GET_HYPERVISOR_CPU_MODEL_NAMES, + (xdrproc_t) xdr_remote_connect_get_hypervisor_cpu_model_names_args, + (char *) &args, + (xdrproc_t) xdr_remote_connect_get_hypervisor_cpu_model_names_ret, + (char *) &ret) < 0) + goto done; + + /* Check the length of the returned list carefully. */ + if (ret.names.names_len > REMOTE_CONNECT_CPU_MODELS_MAX) { + virReportError(VIR_ERR_RPC, + _("Too many model names '%d' for limit '%d'"), + ret.names.names_len, + REMOTE_CONNECT_CPU_MODELS_MAX); + goto cleanup; + }
Also validate ret.names.names_len == ret.aliases.aliases_len
+ + *names = g_new0(char *, ret.names.names_len + 1); + for (i = 0; i < ret.names.names_len; i++) { + (*names)[i] = g_steal_pointer(&ret.names.names_val[i]); + } + + *aliases = g_new0(char *, ret.aliases.aliases_len + 1); + for (i = 0; i < ret.aliases.aliases_len; i++) { + (*aliases)[i] = g_steal_pointer(&ret.aliases.aliases_val[i]); + } + + rv = ret.ret; + + cleanup: + xdr_free((xdrproc_t) xdr_remote_connect_get_hypervisor_cpu_model_names_ret, + (char *) &ret); + + done: + remoteDriverUnlock(priv); + return rv; +} +
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 79ffc63f03..a5c60399c7 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3303,6 +3303,17 @@ struct remote_connect_get_cpu_model_names_ret { int ret; };
+struct remote_connect_get_hypervisor_cpu_model_names_args { + remote_nonnull_string arch; + unsigned int flags; +}; + +struct remote_connect_get_hypervisor_cpu_model_names_ret { + remote_nonnull_string names<REMOTE_CONNECT_CPU_MODELS_MAX>; + remote_nonnull_string aliases<REMOTE_CONNECT_CPU_MODELS_MAX>;
I'd expect 'remote_string', since aliases should be able to be NULL
+ int ret;
Drop 'ret'
+}; +
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3b5c3db67c..538a35d327 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17311,6 +17311,57 @@ qemuConnectGetCPUModelNames(virConnectPtr conn, return virCPUGetModels(arch, models); } +static int +qemuConnectGetHypervisorCPUModelNames(virConnectPtr conn, + const char *archName, + char ***names, + char ***aliases, + unsigned int flags) +{ + size_t i; + virQEMUDriver *driver = conn->privateData; + g_autoptr(qemuMonitorCPUDefs) cpuDefs = NULL; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autoptr(qemuProcessQMP) proc = NULL; + g_autoptr(virQEMUCaps) qemuCaps = NULL; + + virCheckFlags(0, -1); + if (virConnectGetHypervisorCPUModelNamesEnsureACL(conn) < 0) + return -1; + + qemuCaps = virQEMUCapsCacheLookupDefault(driver->qemuCapsCache, NULL, + archName, NULL, NULL, NULL, + NULL, NULL); + + if (!(proc = qemuProcessQMPNew(virQEMUCapsGetBinary(qemuCaps), cfg->libDir, + cfg->user, cfg->group, false))) + return -1; + + if (qemuProcessQMPStart(proc) < 0) + return -1; + + if (qemuMonitorGetCPUDefinitions(proc->mon, &cpuDefs) < 0) + return -1; + + // plus one to NULL terminate the lists + *names = g_new(char*, cpuDefs->ncpus + 1); + *aliases = g_new(char*, cpuDefs->ncpus + 1); + + for (i = 0; i < cpuDefs->ncpus; ++i) { + if (cpuDefs->cpus[i].name) + (*names)[i] = g_strdup(cpuDefs->cpus[i].name); + else + (*names)[i] = g_strdup(""); + + if (cpuDefs->cpus[i].alias) + (*aliases)[i] = g_strdup(cpuDefs->cpus[i].alias); + else + (*aliases)[i] = g_strdup(""); + } + + return cpuDefs->ncpus; +} + static int qemuDomainGetHostnameAgent(virQEMUDriver *driver, @@ -21324,6 +21375,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainGetMessages = qemuDomainGetMessages, /* 7.1.0 */ .domainStartDirtyRateCalc = qemuDomainStartDirtyRateCalc, /* 7.2.0 */ .domainSetLaunchSecurityState = qemuDomainSetLaunchSecurityState, /* 8.0.0 */ + .connectGetHypervisorCPUModelNames = qemuConnectGetHypervisorCPUModelNames, /* 8.5.0 */ }; -- 2.31.1
On Tue, Jun 28, 2022 at 06:09:41PM +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3b5c3db67c..538a35d327 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17311,6 +17311,57 @@ qemuConnectGetCPUModelNames(virConnectPtr conn, return virCPUGetModels(arch, models); }
+static int +qemuConnectGetHypervisorCPUModelNames(virConnectPtr conn, + const char *archName, + char ***names, + char ***aliases, + unsigned int flags) +{ + size_t i; + virQEMUDriver *driver = conn->privateData; + g_autoptr(qemuMonitorCPUDefs) cpuDefs = NULL; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autoptr(qemuProcessQMP) proc = NULL; + g_autoptr(virQEMUCaps) qemuCaps = NULL; + + virCheckFlags(0, -1); + if (virConnectGetHypervisorCPUModelNamesEnsureACL(conn) < 0) + return -1; + + qemuCaps = virQEMUCapsCacheLookupDefault(driver->qemuCapsCache, NULL, + archName, NULL, NULL, NULL, + NULL, NULL); + + if (!(proc = qemuProcessQMPNew(virQEMUCapsGetBinary(qemuCaps), cfg->libDir, + cfg->user, cfg->group, false))) + return -1; + + if (qemuProcessQMPStart(proc) < 0) + return -1; + + if (qemuMonitorGetCPUDefinitions(proc->mon, &cpuDefs) < 0) + return -1; + + // plus one to NULL terminate the lists + *names = g_new(char*, cpuDefs->ncpus + 1); + *aliases = g_new(char*, cpuDefs->ncpus + 1); + + for (i = 0; i < cpuDefs->ncpus; ++i) { + if (cpuDefs->cpus[i].name) + (*names)[i] = g_strdup(cpuDefs->cpus[i].name); + else + (*names)[i] = g_strdup("");
Why would there be a case where 'name' is NULL ? I would have thought that's an error scenario and qemu_monitor_json.c should have reported a fatal error for that ? Or am i missing something ?
+ + if (cpuDefs->cpus[i].alias) + (*aliases)[i] = g_strdup(cpuDefs->cpus[i].alias); + else + (*aliases)[i] = g_strdup("");
I'd expect us to use NULL for the "no alias" marker. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
$ virsh hypervisor-cpu-models --arch x86_64 Name Alias ---------------------------------------------------- max host base qemu64-v1 qemu64 qemu64-v1 qemu32-v1 qemu32 qemu32-v1 phenom-v1 phenom phenom-v1 (...) Note that this includes alias names and cpu models that currently do not exist in libvirt, e.g. Denverton and Knights Mill. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- docs/manpages/virsh.rst | 14 +++++++++ tools/virsh-host.c | 67 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 45469f2f35..a55792b0e2 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -955,6 +955,20 @@ If *--validate* is specified, validates the format of the XML document against an internal RNG schema. +hypervisor-cpu-models +--------------------- + +**Syntax:** + +:: + + hypervisor-cpu-models [arch] + +List the names of cpu models known to the hypervisor. A hypervisor may define +alias names for some or all cpu models. Note that the cpu models may differ +from libvirt, even if named identically. + + hypervisor-cpu-baseline ----------------------- diff --git a/tools/virsh-host.c b/tools/virsh-host.c index ead966b500..cb8e1e8c6d 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -32,6 +32,7 @@ #include "virstring.h" #include "virfile.h" #include "virenum.h" +#include "vsh-table.h" /* * "capabilities" command @@ -1674,6 +1675,65 @@ cmdHypervisorCPUCompare(vshControl *ctl, } +/* + * "hypervisor-cpu-models" command + */ +static const vshCmdInfo info_hypervisor_cpu_models[] = { + {.name = "help", + .data = N_("return list of CPU models supported by a specific hypervisor") + }, + {.name = "desc", + .data = N_("Return list of CPU models supported by a specific hypervisor") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_hypervisor_cpu_models[] = { + {.name = "arch", + .type = VSH_OT_STRING, + .completer = virshArchCompleter, + .help = N_("CPU architecture (/domain/os/type/@arch)"), + }, + {.name = NULL} +}; + +static bool +cmdHypervisorCPUModels(vshControl *ctl, + const vshCmd *cmd) +{ + virshControl *priv = ctl->privData; + bool ret = false; + const char *arch = NULL; + char **name = NULL; + char **alias = NULL; + int nresults; + g_autoptr(vshTable) table = vshTableNew(_("Name"), _("Alias"), NULL); + + if (!table) + return ret; + + if (vshCommandOptStringReq(ctl, cmd, "arch", &arch) < 0) + return false; + + nresults = virConnectGetHypervisorCPUModelNames(priv->conn, arch, &name, + &alias, 0); + if (nresults >= 0) { + size_t index; + for (index = 0; index < nresults; ++index) { + if (vshTableRowAppend(table, name[index], alias[index], NULL) < 0) + return ret; + g_free(name[index]); + g_free(alias[index]); + } + ret = true; + } + vshTablePrintToStdout(table, ctl); + g_free(name); + g_free(alias); + + return ret; +} + /* * "hypervisor-cpu-baseline" command */ @@ -1819,6 +1879,13 @@ const vshCmdDef hostAndHypervisorCmds[] = { .info = info_hostname, .flags = 0 }, + { + .name = "hypervisor-cpu-models", + .handler = cmdHypervisorCPUModels, + .opts = opts_hypervisor_cpu_models, + .info = info_hypervisor_cpu_models, + .flags = 0 + }, {.name = "hypervisor-cpu-baseline", .handler = cmdHypervisorCPUBaseline, .opts = opts_hypervisor_cpu_baseline, -- 2.31.1
On Tue, Jun 28, 2022 at 06:09:42PM +0200, Tim Wiederhake wrote:
$ virsh hypervisor-cpu-models --arch x86_64 Name Alias ---------------------------------------------------- max host base qemu64-v1 qemu64 qemu64-v1 qemu32-v1 qemu32 qemu32-v1 phenom-v1 phenom phenom-v1 (...)
Note that this includes alias names and cpu models that currently do not exist in libvirt, e.g. Denverton and Knights Mill.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- docs/manpages/virsh.rst | 14 +++++++++ tools/virsh-host.c | 67 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 45469f2f35..a55792b0e2 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -955,6 +955,20 @@ If *--validate* is specified, validates the format of the XML document against an internal RNG schema.
+hypervisor-cpu-models +--------------------- + +**Syntax:** + +:: + + hypervisor-cpu-models [arch] + +List the names of cpu models known to the hypervisor. A hypervisor may define +alias names for some or all cpu models. Note that the cpu models may differ +from libvirt, even if named identically. + + hypervisor-cpu-baseline -----------------------
diff --git a/tools/virsh-host.c b/tools/virsh-host.c index ead966b500..cb8e1e8c6d 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -32,6 +32,7 @@ #include "virstring.h" #include "virfile.h" #include "virenum.h" +#include "vsh-table.h"
/* * "capabilities" command @@ -1674,6 +1675,65 @@ cmdHypervisorCPUCompare(vshControl *ctl, }
+/* + * "hypervisor-cpu-models" command + */ +static const vshCmdInfo info_hypervisor_cpu_models[] = { + {.name = "help", + .data = N_("return list of CPU models supported by a specific hypervisor") + }, + {.name = "desc", + .data = N_("Return list of CPU models supported by a specific hypervisor") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_hypervisor_cpu_models[] = { + {.name = "arch", + .type = VSH_OT_STRING, + .completer = virshArchCompleter, + .help = N_("CPU architecture (/domain/os/type/@arch)"), + }, + {.name = NULL} +};
This will need to be expanded to add machine, virttype and emulator to match the API params.
+ nresults = virConnectGetHypervisorCPUModelNames(priv->conn, arch, &name, + &alias, 0);
Should we do sorting here or somewhere earlier in the flow ? I notice the existing 'cpu-models' doesn't do sorting just returning stuff in a pretty horrible random order. I think both this new API and the old one should sort case insensitive.
+ if (nresults >= 0) { + size_t index; + for (index = 0; index < nresults; ++index) { + if (vshTableRowAppend(table, name[index], alias[index], NULL) < 0) + return ret; + g_free(name[index]); + g_free(alias[index]); + } + ret = true; + } + vshTablePrintToStdout(table, ctl); + g_free(name); + g_free(alias); + + return ret; +} + /* * "hypervisor-cpu-baseline" command */ @@ -1819,6 +1879,13 @@ const vshCmdDef hostAndHypervisorCmds[] = { .info = info_hostname, .flags = 0 }, + { + .name = "hypervisor-cpu-models", + .handler = cmdHypervisorCPUModels, + .opts = opts_hypervisor_cpu_models, + .info = info_hypervisor_cpu_models, + .flags = 0 + }, {.name = "hypervisor-cpu-baseline", .handler = cmdHypervisorCPUBaseline, .opts = opts_hypervisor_cpu_baseline, -- 2.31.1
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- include/libvirt/libvirt-host.h | 7 +++++ src/driver-hypervisor.h | 9 ++++++ src/libvirt-host.c | 54 ++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 71 insertions(+) diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index 5aaa001adb..ad11c1172d 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -968,6 +968,13 @@ int virConnectGetHypervisorCPUModelNames(virConnectPtr conn, char ***aliases, unsigned int flags); +int virConnectGetHypervisorCPUModelDefinition(virConnectPtr conn, + const char *arch, + const char *machine, + const char *name, + char **xmlCPU, + unsigned int flags); + /** * virConnectBaselineCPUFlags: * diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index c81e5d4c75..b016ec7775 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -739,6 +739,14 @@ typedef int char ***aliases, unsigned int flags); +typedef int +(*virDrvConnectGetHypervisorCPUModelDefinition)(virConnectPtr conn, + const char *arch, + const char *machine, + const char *name, + char **xmlCPU, + unsigned int flags); + typedef int (*virDrvDomainGetJobInfo)(virDomainPtr domain, virDomainJobInfoPtr info); @@ -1720,4 +1728,5 @@ struct _virHypervisorDriver { virDrvDomainGetMessages domainGetMessages; virDrvDomainStartDirtyRateCalc domainStartDirtyRateCalc; virDrvConnectGetHypervisorCPUModelNames connectGetHypervisorCPUModelNames; + virDrvConnectGetHypervisorCPUModelDefinition connectGetHypervisorCPUModelDefinition; }; diff --git a/src/libvirt-host.c b/src/libvirt-host.c index 6e734628c1..8875f67a7b 100644 --- a/src/libvirt-host.c +++ b/src/libvirt-host.c @@ -1288,6 +1288,60 @@ virConnectGetHypervisorCPUModelNames(virConnectPtr conn, return -1; } +/** + * virConnectGetHypervisorCPUModelDefinition: + * + * @conn: virConnect connection + * @arch: Architecture + * @machine: Machine type + * @name: CPU model name + * @xmlCPU: XML description of the CPUs + * @flags: extra flags; not used yet, so callers should always pass 0. + * + * Get the description of a specific hypervisor cpu model for a given machine + * type and architecture. + * + * Returns 0 on success, -1 otherwise. + * + * Since: 8.5.0 + */ +int +virConnectGetHypervisorCPUModelDefinition(virConnectPtr conn, + const char *arch, + const char *machine, + const char *name, + char **xmlCPU, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, arch=%s, machine=%s, model=%s, flags=0x%x", conn, + NULLSTR(arch), NULLSTR(machine), NULLSTR(name), flags); + virResetLastError(); + + virCheckConnectReturn(conn, -1); + virCheckNonNullArgGoto(arch, error); + virCheckNonNullArgGoto(machine, error); + virCheckNonNullArgGoto(name, error); + + if (conn->driver->connectGetHypervisorCPUModelDefinition) { + int ret; + + ret = conn->driver->connectGetHypervisorCPUModelDefinition(conn, arch, + machine, + name, + xmlCPU, + flags); + if (ret < 0) + goto error; + + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(conn); + return -1; +} /** * virConnectBaselineCPU: diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index c6a8e898ae..15ef29d07d 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -924,6 +924,7 @@ LIBVIRT_8.4.0 { LIBVIRT_8.5.0 { global: + virConnectGetHypervisorCPUModelDefinition; virConnectGetHypervisorCPUModelNames; virDomainAbortJobFlags; } LIBVIRT_8.4.0; -- 2.31.1
On Tue, Jun 28, 2022 at 06:09:43PM +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- include/libvirt/libvirt-host.h | 7 +++++ src/driver-hypervisor.h | 9 ++++++ src/libvirt-host.c | 54 ++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 71 insertions(+)
diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index 5aaa001adb..ad11c1172d 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -968,6 +968,13 @@ int virConnectGetHypervisorCPUModelNames(virConnectPtr conn, char ***aliases, unsigned int flags);
+int virConnectGetHypervisorCPUModelDefinition(virConnectPtr conn, + const char *arch, + const char *machine,
Now we've got machine + arch, but still haven't got 'virttype' and 'emulator' parameters
+ const char *name, + char **xmlCPU, + unsigned int flags); +
diff --git a/src/libvirt-host.c b/src/libvirt-host.c index 6e734628c1..8875f67a7b 100644 --- a/src/libvirt-host.c +++ b/src/libvirt-host.c @@ -1288,6 +1288,60 @@ virConnectGetHypervisorCPUModelNames(virConnectPtr conn, return -1; }
+/** + * virConnectGetHypervisorCPUModelDefinition: + * + * @conn: virConnect connection + * @arch: Architecture + * @machine: Machine type
Should be optional, defaulting to the default machine type if omitted. For that matter, arch should be optional too
+ * @name: CPU model name + * @xmlCPU: XML description of the CPUs + * @flags: extra flags; not used yet, so callers should always pass 0. + * + * Get the description of a specific hypervisor cpu model for a given machine + * type and architecture. + * + * Returns 0 on success, -1 otherwise. + * + * Since: 8.5.0 + */ +int +virConnectGetHypervisorCPUModelDefinition(virConnectPtr conn, + const char *arch, + const char *machine, + const char *name, + char **xmlCPU, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, arch=%s, machine=%s, model=%s, flags=0x%x", conn, + NULLSTR(arch), NULLSTR(machine), NULLSTR(name), flags); + virResetLastError(); + + virCheckConnectReturn(conn, -1); + virCheckNonNullArgGoto(arch, error); + virCheckNonNullArgGoto(machine, error); + virCheckNonNullArgGoto(name, error); + + if (conn->driver->connectGetHypervisorCPUModelDefinition) { + int ret; + + ret = conn->driver->connectGetHypervisorCPUModelDefinition(conn, arch, + machine, + name, + xmlCPU, + flags); + if (ret < 0) + goto error; + + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(conn); + return -1; +}
/** * virConnectBaselineCPU: diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index c6a8e898ae..15ef29d07d 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -924,6 +924,7 @@ LIBVIRT_8.4.0 {
LIBVIRT_8.5.0 { global: + virConnectGetHypervisorCPUModelDefinition; virConnectGetHypervisorCPUModelNames; virDomainAbortJobFlags; } LIBVIRT_8.4.0; -- 2.31.1
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, Jul 18, 2022 at 11:26:07AM +0100, Daniel P. Berrangé wrote:
On Tue, Jun 28, 2022 at 06:09:43PM +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- include/libvirt/libvirt-host.h | 7 +++++ src/driver-hypervisor.h | 9 ++++++ src/libvirt-host.c | 54 ++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 71 insertions(+)
diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index 5aaa001adb..ad11c1172d 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -968,6 +968,13 @@ int virConnectGetHypervisorCPUModelNames(virConnectPtr conn, char ***aliases, unsigned int flags);
+int virConnectGetHypervisorCPUModelDefinition(virConnectPtr conn, + const char *arch, + const char *machine,
Now we've got machine + arch, but still haven't got 'virttype' and 'emulator' parameters
+ const char *name, + char **xmlCPU, + unsigned int flags); +
I'm not actually convinced this new API is needed at all. Thue virConnectBaselineHypervisorCPU API has a flag VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES to let you expand all features in a given CPU model name. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/remote/remote_daemon_dispatch.c | 32 ++++++++++++++++++++++ src/remote/remote_driver.c | 41 +++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 20 +++++++++++++- src/remote_protocol-structs | 11 ++++++++ 4 files changed, 103 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 9011977e18..bc30cb819c 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -5913,6 +5913,38 @@ remoteDispatchConnectGetHypervisorCPUModelNames(virNetServer *server G_GNUC_UNUS } +static int +remoteDispatchConnectGetHypervisorCPUModelDefinition(virNetServer *server G_GNUC_UNUSED, + virNetServerClient *client, + virNetMessage *msg G_GNUC_UNUSED, + struct virNetMessageError *rerr, + remote_connect_get_hypervisor_cpu_model_definition_args *args, + remote_connect_get_hypervisor_cpu_model_definition_ret *ret) +{ + int rv = -1; + virConnectPtr conn = remoteGetHypervisorConn(client); + g_autofree char *xml = NULL; + + if (!conn) + goto cleanup; + + rv = virConnectGetHypervisorCPUModelDefinition(conn, args->arch, + args->machine, args->name, + &xml, args->flags); + if (rv < 0) + goto cleanup; + + ret->xml = g_steal_pointer(&xml); + rv = 0; + + cleanup: + ret->ret = rv; + if (rv < 0) + virNetMessageSaveError(rerr); + return rv; +} + + static int remoteDispatchDomainCreateXMLWithFiles(virNetServer *server G_GNUC_UNUSED, virNetServerClient *client, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 8dca23a7eb..7aec988746 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6451,6 +6451,46 @@ remoteConnectGetHypervisorCPUModelNames(virConnectPtr conn, } +static int +remoteConnectGetHypervisorCPUModelDefinition(virConnectPtr conn, + const char *arch, + const char *machine, + const char *name, + char **xmlCPU, + unsigned int flags) +{ + int rv = -1; + remote_connect_get_hypervisor_cpu_model_definition_args args; + remote_connect_get_hypervisor_cpu_model_definition_ret ret; + + struct private_data *priv = conn->privateData; + + remoteDriverLock(priv); + + args.arch = (char *) arch; + args.machine = (char *) machine; + args.name = (char *) name; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + if (call(conn, priv, 0, REMOTE_PROC_CONNECT_GET_HYPERVISOR_CPU_MODEL_DEFINITION, + (xdrproc_t) xdr_remote_connect_get_hypervisor_cpu_model_definition_args, + (char *) &args, + (xdrproc_t) xdr_remote_connect_get_hypervisor_cpu_model_definition_ret, + (char *) &ret) < 0) + goto done; + + *xmlCPU = g_steal_pointer(&ret.xml); + rv = ret.ret; + + xdr_free((xdrproc_t) xdr_remote_connect_get_hypervisor_cpu_model_definition_ret, + (char *) &ret); + + done: + remoteDriverUnlock(priv); + return rv; +} + static int remoteDomainOpenGraphics(virDomainPtr dom, unsigned int idx, @@ -8711,6 +8751,7 @@ static virHypervisorDriver hypervisor_driver = { .domainStartDirtyRateCalc = remoteDomainStartDirtyRateCalc, /* 7.2.0 */ .domainSetLaunchSecurityState = remoteDomainSetLaunchSecurityState, /* 8.0.0 */ .connectGetHypervisorCPUModelNames = remoteConnectGetHypervisorCPUModelNames, /* 8.5.0 */ + .connectGetHypervisorCPUModelDefinition = remoteConnectGetHypervisorCPUModelDefinition, /* 8.5.0 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index a5c60399c7..2d7d6277c6 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3314,6 +3314,18 @@ struct remote_connect_get_hypervisor_cpu_model_names_ret { int ret; }; +struct remote_connect_get_hypervisor_cpu_model_definition_args { + remote_nonnull_string arch; + remote_nonnull_string machine; + remote_nonnull_string name; + unsigned int flags; +}; + +struct remote_connect_get_hypervisor_cpu_model_definition_ret { + remote_nonnull_string xml; + int ret; +}; + struct remote_connect_network_event_register_any_args { int eventID; remote_network net; @@ -6976,5 +6988,11 @@ enum remote_procedure { * @generate: none * @acl: connect:read */ - REMOTE_PROC_CONNECT_GET_HYPERVISOR_CPU_MODEL_NAMES = 443 + REMOTE_PROC_CONNECT_GET_HYPERVISOR_CPU_MODEL_NAMES = 443, + + /** + * @generate: none + * @acl: connect:read + */ + REMOTE_PROC_CONNECT_GET_HYPERVISOR_CPU_MODEL_DEFINITION = 444 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index c6afb92aad..0fdc1e8800 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2686,6 +2686,16 @@ struct remote_connect_get_hypervisor_cpu_model_names_ret { } aliases; int ret; }; +struct remote_connect_get_hypervisor_cpu_model_definition_args { + remote_nonnull_string arch; + remote_nonnull_string machine; + remote_nonnull_string name; + u_int flags; +}; +struct remote_connect_get_hypervisor_cpu_model_definition_ret { + remote_nonnull_string xml; + int ret; +}; struct remote_connect_network_event_register_any_args { int eventID; remote_network net; @@ -3727,4 +3737,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_RESTORE_PARAMS = 441, REMOTE_PROC_DOMAIN_ABORT_JOB_FLAGS = 442, REMOTE_PROC_CONNECT_GET_HYPERVISOR_CPU_MODEL_NAMES = 443, + REMOTE_PROC_CONNECT_GET_HYPERVISOR_CPU_MODEL_DEFINITION = 444, }; -- 2.31.1
On Tue, Jun 28, 2022 at 06:09:44PM +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/remote/remote_daemon_dispatch.c | 32 ++++++++++++++++++++++ src/remote/remote_driver.c | 41 +++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 20 +++++++++++++- src/remote_protocol-structs | 11 ++++++++ 4 files changed, 103 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index a5c60399c7..2d7d6277c6 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3314,6 +3314,18 @@ struct remote_connect_get_hypervisor_cpu_model_names_ret { int ret; };
+struct remote_connect_get_hypervisor_cpu_model_definition_args { + remote_nonnull_string arch; + remote_nonnull_string machine; + remote_nonnull_string name; + unsigned int flags; +}; + +struct remote_connect_get_hypervisor_cpu_model_definition_ret { + remote_nonnull_string xml; + int ret; +};
'ret' should not be here, as errors are encoded separately on the wire. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_driver.c | 53 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 538a35d327..0dfc93a373 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17363,6 +17363,58 @@ qemuConnectGetHypervisorCPUModelNames(virConnectPtr conn, } +static int +qemuConnectGetHypervisorCPUModelDefinition(virConnectPtr conn, + const char *arch, + const char *machine, + const char *name, + char **xmlCPU, + unsigned int flags) +{ + virQEMUDriver *driver = conn->privateData; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autoptr(qemuProcessQMP) proc = NULL; + g_autoptr(virQEMUCaps) qemuCaps = NULL; + g_autoptr(virCPUDef) cpu = g_new0(virCPUDef, 1); + g_autoptr(qemuMonitorCPUModelInfo) model_info = NULL; + + virCheckFlags(0, -1); + if (virConnectGetHypervisorCPUModelDefinitionEnsureACL(conn) < 0) + return -1; + + qemuCaps = virQEMUCapsCacheLookupDefault(driver->qemuCapsCache, NULL, + arch, NULL, machine, NULL, + NULL, NULL); + if (!qemuCaps) + return -1; + + if (!(proc = qemuProcessQMPNew(virQEMUCapsGetBinary(qemuCaps), cfg->libDir, + cfg->user, cfg->group, false))) + return -1; + + if (qemuProcessQMPStart(proc) < 0) + return -1; + + cpu->model = g_strdup(name); + if (qemuMonitorGetCPUModelExpansion(proc->mon, + QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL, + cpu, true, true, &model_info) < 0) + return -1; + + if (!model_info) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown model '%s'"), name); + return -1; + } + + if (qemuConnectStealCPUModelFromInfo(cpu, &model_info) < 0) + return -1; + + *xmlCPU = virCPUDefFormat(cpu, NULL); + return 0; +} + + static int qemuDomainGetHostnameAgent(virQEMUDriver *driver, virDomainObj *vm, @@ -21376,6 +21428,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainStartDirtyRateCalc = qemuDomainStartDirtyRateCalc, /* 7.2.0 */ .domainSetLaunchSecurityState = qemuDomainSetLaunchSecurityState, /* 8.0.0 */ .connectGetHypervisorCPUModelNames = qemuConnectGetHypervisorCPUModelNames, /* 8.5.0 */ + .connectGetHypervisorCPUModelDefinition = qemuConnectGetHypervisorCPUModelDefinition, /* 8.5.0 */ }; -- 2.31.1
$ virsh hypervisor-cpu-definition --machine pc --arch x86_64 qemu64 <cpu> <model>qemu64</model> <feature name='cmov'/> <feature name='mmx'/> <feature name='xd'/> <feature name='x-intel-pt-auto-level'/> <feature name='kvm_asyncpf'/> <feature name='kvm-asyncpf'/> <feature name='legacy-cache'/> <feature name='vmware-cpuid-freq'/> <feature name='mce'/> <feature name='mca'/> <feature name='msr'/> <feature name='fxsr'/> <feature name='cpuid-0xb'/> <feature name='kvm_pv_eoi'/> <feature name='pni'/> <feature name='x2apic'/> <feature name='i64'/> <feature name='pae'/> <feature name='pat'/> <feature name='sse'/> <feature name='kvm_nopiodelay'/> <feature name='kvm-nopiodelay'/> <feature name='kvmclock-stable-bit'/> <feature name='hypervisor'/> <feature name='syscall'/> <feature name='x-migrate-smi-count'/> <feature name='full-cpuid-auto-level'/> <feature name='sse3'/> <feature name='sse2'/> <feature name='kvm-pv-eoi'/> <feature name='cx8'/> <feature name='pge'/> <feature name='fill-mtrr-mask'/> <feature name='cx16'/> <feature name='de'/> <feature name='clflush'/> <feature name='tsc'/> <feature name='fpu'/> <feature name='check'/> <feature name='apic'/> <feature name='kvm-steal-time'/> <feature name='kvm_steal_time'/> <feature name='kvmclock'/> <feature name='l3-cache'/> <feature name='nx'/> <feature name='tcg-cpuid'/> <feature name='lm'/> <feature name='pse'/> <feature name='sep'/> <feature name='kvm'/> <feature name='lahf-lm'/> <feature name='lahf_lm'/> <feature name='mtrr'/> <feature name='pse36'/> </cpu> Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- docs/manpages/virsh.rst | 13 +++++++++ tools/virsh-host.c | 65 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index a55792b0e2..aa6c732fd0 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -969,6 +969,19 @@ alias names for some or all cpu models. Note that the cpu models may differ from libvirt, even if named identically. +hypervisor-cpu-definition +------------------------- + +**Syntax:** + +:: + + hypervisor-cpu-definition name [arch] [machine] + +Retrieve the named CPU model as defined by the hypervisor. Note that the cpu +model may differ from libvirt, even if named identically. + + hypervisor-cpu-baseline ----------------------- diff --git a/tools/virsh-host.c b/tools/virsh-host.c index cb8e1e8c6d..75b439d04b 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -1734,6 +1734,64 @@ cmdHypervisorCPUModels(vshControl *ctl, return ret; } +/* + * "hypervisor-cpu-definition" command + */ +static const vshCmdInfo info_hypervisor_cpu_definition[] = { + {.name = "help", + .data = N_("return CPU model as defined by hypervisor") + }, + {.name = "desc", + .data = N_("Return CPU model as defined by hypervisor") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_hypervisor_cpu_definition[] = { + {.name = "name", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("CPU name"), + }, + {.name = "arch", + .type = VSH_OT_STRING, + .completer = virshArchCompleter, + .help = N_("CPU architecture (/domain/os/type/@arch)"), + }, + {.name = "machine", + .type = VSH_OT_STRING, + .help = N_("machine type (/domain/os/type/@machine)"), + }, + {.name = NULL} +}; + +static bool +cmdHypervisorCPUDefinition(vshControl *ctl, + const vshCmd *cmd) +{ + virshControl *priv = ctl->privData; + const char *arch = NULL; + const char *machine = NULL; + const char *name = NULL; + g_autofree char *xml = NULL; + + if (vshCommandOptStringReq(ctl, cmd, "arch", &arch) < 0) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "machine", &machine) < 0) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0) + return false; + + if (virConnectGetHypervisorCPUModelDefinition(priv->conn, arch, machine, + name, &xml, 0) < 0) + return false; + + vshPrint(ctl, "%s", xml); + return true; +} + /* * "hypervisor-cpu-baseline" command */ @@ -1886,6 +1944,13 @@ const vshCmdDef hostAndHypervisorCmds[] = { .info = info_hypervisor_cpu_models, .flags = 0 }, + { + .name = "hypervisor-cpu-definition", + .handler = cmdHypervisorCPUDefinition, + .opts = opts_hypervisor_cpu_definition, + .info = info_hypervisor_cpu_definition, + .flags = 0 + }, {.name = "hypervisor-cpu-baseline", .handler = cmdHypervisorCPUBaseline, .opts = opts_hypervisor_cpu_baseline, -- 2.31.1
See below. What do the more experienced libvirt developers think, is this going in the right direction? Thanks, Tim On Tue, 2022-06-28 at 18:09 +0200, Tim Wiederhake wrote:
libvirt and qemu cpu models are out of sync. libvirt cpu models are considered static and never changing, whereas qemu cpu models have changed over time. This leads to problems and confusion for users, as the cpu models they specify may have different features than what they expect.
This series introduces two new APIs to enumerate and retrieve cpu models dynamically from the hypervisor at runtime, allowing the user to make use of cpu models from newer hypervisor versions even if libvirt is not aware of them.
These two new functions are intended as building blocks onto which higher-level functionality can be build, e.g. the ability to specify a hypervisor cpu model directly in a domain xml. With these two functions alone this has to be done manually for now, i.e. enumerate the available cpu models, retrieve the cpu description and manually insert it into the domain xml accordingly:
$ virsh hypervisor-cpu-models --arch x86_64 Name Alias ---------------------------------------------------- max host base qemu64-v1 qemu64 qemu64-v1 qemu32-v1 qemu32 qemu32-v1 phenom-v1 phenom phenom-v1 (...)
$ virsh hypervisor-cpu-definition --machine pc --arch x86_64 qemu64 <cpu> <model>qemu64</model> <feature name='cmov'/> <feature name='mmx'/> <feature name='xd'/> <feature name='x-intel-pt-auto-level'/> <feature name='kvm_asyncpf'/> <feature name='kvm-asyncpf'/> <feature name='legacy-cache'/> <feature name='vmware-cpuid-freq'/> <feature name='mce'/> <feature name='mca'/> <feature name='msr'/> <feature name='fxsr'/> <feature name='cpuid-0xb'/> <feature name='kvm_pv_eoi'/> <feature name='pni'/> <feature name='x2apic'/> <feature name='i64'/> <feature name='pae'/> <feature name='pat'/> <feature name='sse'/> <feature name='kvm_nopiodelay'/> <feature name='kvm-nopiodelay'/> <feature name='kvmclock-stable-bit'/> <feature name='hypervisor'/> <feature name='syscall'/> <feature name='x-migrate-smi-count'/> <feature name='full-cpuid-auto-level'/> <feature name='sse3'/> <feature name='sse2'/> <feature name='kvm-pv-eoi'/> <feature name='cx8'/> <feature name='pge'/> <feature name='fill-mtrr-mask'/> <feature name='cx16'/> <feature name='de'/> <feature name='clflush'/> <feature name='tsc'/> <feature name='fpu'/> <feature name='check'/> <feature name='apic'/> <feature name='kvm-steal-time'/> <feature name='kvm_steal_time'/> <feature name='kvmclock'/> <feature name='l3-cache'/> <feature name='nx'/> <feature name='tcg-cpuid'/> <feature name='lm'/> <feature name='pse'/> <feature name='sep'/> <feature name='kvm'/> <feature name='lahf-lm'/> <feature name='lahf_lm'/> <feature name='mtrr'/> <feature name='pse36'/> </cpu>
The returned cpu description is currently completely unfiltered, as can be seen by the duplicate entries differing only in "-" vs. "_" usage, inclusion of experimental feature flags and generally, flags that are not recognized by libvirt. One possibility to adress this would be to extend virQEMUCapsCPUFeatureTranslate.
Before I continue with this, I would love to hear other people's thoughts, comments and potential use cases.
Cheers, Tim
Tim Wiederhake (9): qemuMonitorCPUDefInfo: Add alias libvirt: introduce virConnectGetHypervisorCPUModelNames public API remote: Add RPC support for the virConnectGetHypervisorCPUModelNames API qemu: implement virConnectGetHypervisorCPUModelNames API tools: Report hypervisor cpu model names libvirt: introduce virConnectGetHypervisorCPUModelDefinition public API remote: Add support for the virConnectGetHypervisorCPUModelDefinition API qemu: Implement virConnectGetHypervisorCPUModelDefinition API tools: Report hypervisor cpu model definitions
docs/manpages/virsh.rst | 27 ++++++ include/libvirt/libvirt-host.h | 13 +++ src/driver-hypervisor.h | 17 ++++ src/libvirt-host.c | 109 +++++++++++++++++++++++ src/libvirt_public.syms | 2 + src/qemu/qemu_driver.c | 105 ++++++++++++++++++++++ src/qemu/qemu_monitor.c | 2 + src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 3 + src/remote/remote_daemon_dispatch.c | 76 ++++++++++++++++ src/remote/remote_driver.c | 100 +++++++++++++++++++++ src/remote/remote_protocol.x | 37 +++++++- src/remote_protocol-structs | 27 ++++++ tools/virsh-host.c | 132 ++++++++++++++++++++++++++++ 14 files changed, 650 insertions(+), 1 deletion(-)
-- 2.31.1
On Tue, Jun 28, 2022 at 06:09:37PM +0200, Tim Wiederhake wrote:
libvirt and qemu cpu models are out of sync. libvirt cpu models are considered static and never changing, whereas qemu cpu models have changed over time. This leads to problems and confusion for users, as the cpu models they specify may have different features than what they expect.
This series introduces two new APIs to enumerate and retrieve cpu models dynamically from the hypervisor at runtime, allowing the user to make use of cpu models from newer hypervisor versions even if libvirt is not aware of them.
These two new functions are intended as building blocks onto which higher-level functionality can be build, e.g. the ability to specify a hypervisor cpu model directly in a domain xml. With these two functions alone this has to be done manually for now, i.e. enumerate the available cpu models, retrieve the cpu description and manually insert it into the domain xml accordingly:
This series only reports the new CPU models/features. I'm not seeing any changes to allow the user to actually use them in a new VM, as libvirt rejects models it doesn't know of: # virsh edit demo ... <cpu mode='custom' match='exact' check='partial'> <model fallback='allow'>Nehalem-v1</model> </cpu> # virsh start demo error: Failed to start domain 'demo' error: internal error: Unknown CPU model Nehalem-v1
$ virsh hypervisor-cpu-models --arch x86_64 Name Alias ---------------------------------------------------- max host base qemu64-v1 qemu64 qemu64-v1 qemu32-v1 qemu32 qemu32-v1 phenom-v1 phenom phenom-v1 (...)
$ virsh hypervisor-cpu-definition --machine pc --arch x86_64 qemu64 <cpu> <model>qemu64</model> <feature name='cmov'/> <feature name='mmx'/> <feature name='xd'/> <feature name='x-intel-pt-auto-level'/> <feature name='kvm_asyncpf'/> <feature name='kvm-asyncpf'/> <feature name='legacy-cache'/> <feature name='vmware-cpuid-freq'/> <feature name='mce'/> <feature name='mca'/> <feature name='msr'/> <feature name='fxsr'/> <feature name='cpuid-0xb'/> <feature name='kvm_pv_eoi'/> <feature name='pni'/> <feature name='x2apic'/> <feature name='i64'/> <feature name='pae'/> <feature name='pat'/> <feature name='sse'/> <feature name='kvm_nopiodelay'/> <feature name='kvm-nopiodelay'/> <feature name='kvmclock-stable-bit'/> <feature name='hypervisor'/> <feature name='syscall'/> <feature name='x-migrate-smi-count'/> <feature name='full-cpuid-auto-level'/> <feature name='sse3'/> <feature name='sse2'/> <feature name='kvm-pv-eoi'/> <feature name='cx8'/> <feature name='pge'/> <feature name='fill-mtrr-mask'/> <feature name='cx16'/> <feature name='de'/> <feature name='clflush'/> <feature name='tsc'/> <feature name='fpu'/> <feature name='check'/> <feature name='apic'/> <feature name='kvm-steal-time'/> <feature name='kvm_steal_time'/> <feature name='kvmclock'/> <feature name='l3-cache'/> <feature name='nx'/> <feature name='tcg-cpuid'/> <feature name='lm'/> <feature name='pse'/> <feature name='sep'/> <feature name='kvm'/> <feature name='lahf-lm'/> <feature name='lahf_lm'/> <feature name='mtrr'/> <feature name='pse36'/> </cpu>
The returned cpu description is currently completely unfiltered, as can be seen by the duplicate entries differing only in "-" vs. "_" usage, inclusion of experimental feature flags and generally, flags that are not recognized by libvirt. One possibility to adress this would be to extend virQEMUCapsCPUFeatureTranslate.
The duplicated feature names are really bad IMHO, as as the (few) cases where QEMU names diverge from libvirt names. I don't mind us exposing CPU versions that we don't have in our XML database, but directly exposing new features and base CPU models IMHO is not desirable. A really key point of libvirt is to define a (thin) abstraction over the hypervisor, not blindly expose its low level platform specific terminology.
Before I continue with this, I would love to hear other people's thoughts, comments and potential use cases.
Before we actually add these APIs we need to actually be able to use the resulting CPU models to run guests, otherwise there's nothing useful that can be done with this new information reported. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrangé -
Tim Wiederhake