[libvirt] [RFC 0/5] Qemu: s390: Cpu Model Support
Thanks for all your feedback up to this point. We've made some progress on this and we also have a few questions. Please let us know if we're on the right track or if we're off in the weeds on this. Here is what we have so far. Patch #1 updates s390's cpu driver to support some operations needed for cpu model support within Libvirt's existing infrastructure. Patch #2 removes s390's hard coded "host" model string. Jiri thought it made more sense to simply leave this blank. Path #3 adds qmp query-cpu-model-expansion command. Patch #4 probes Qemu's view of the host model (query-cpu-model-expansion), caches the response and uses it to fill in qemuCaps->hostCpuModel. Archs that do not implement query-cpu-model-expansion will continue to fill in qemuCaps->hostCpuModel from caps->host.cpu. Patch #5 updates qemuBuildCpuModelArgStr's VIR_CPU_MODE_HOST_MODEL case. It seems like all archs except PPC64 do not support host model mode. We add an s390 path here and get the guest cpu model from virQEMUCapsGetHostModel() which will get the model name from qemuCaps->hostCpuModel. NOTE: I just realized we'll need need to handle the case where virQEMUCapsGetHostModel returns NULL or the model is NULL or "". We probably should just throw an error and give up in that case. Here is a list of outstanding things to do and some questions. 1. virsh domcapabilities --emulatorbin flag This works today if the command is passed --emulatorbin /usr/bin/qemu-kvm. If no emulatorbin is given then Libvirt seems to choose /usr/bin/qemu-system-s390x my system which does not have kvm enabled. A kvm session is needed for s390 to properly compute the host model. A kvm session is only available for the qemu-kvm binary. Is the answer to tell our users to always supply --emulatorbin with /usr/bin/qemu-kvm argument? Or is there a more user friendly solution to this? 2. virsh baseline and compare support. Both of these commands, for s390 at least, will need to spin up a Qemu session with monitor to compute the result. The only place I see this done today is qemu_capabilities. Rather than blindly duplicate the code, I guess we should carve out some type of api for spinning up a monitor backed Qemu instance, yes? Also, neither compare nor baseline have the --emulatorbin flag. So we'll either need to add them or find an alternate solution. Any thoughts on this? 3.Support host passthrough mode. This essentially just means passing -cpu host to qemu. We know Qemu supports this for s390 today. But Libvirt claims it is not supported due to the following: In virQEMUCapsIsCPUModeSupported, our virt "type" is VIR_DOMAIN_VIRT_QEMU, and not VIR_DOMAIN_VIRT_KVM. virttype comes from domCaps and is set here: qemuConnectGetDomainCapabilities() The relevant code is: if (qemuHostdevHostSupportsPassthroughLegacy() || qemuHostdevHostSupportsPassthroughVFIO()) virttype = VIR_DOMAIN_VIRT_KVM; else virttype = VIR_DOMAIN_VIRT_QEMU; I'll admit, I have not been able to figure out why these checks are failing in my test environment, when I suspect they should be passing. But in my case it seems as though these relate to host device passthrough. How do they apply to cpu model passthrough. What am I missing? Thanks for your time. Collin L. Walling (3): s390: Report blank host model instead of "host" qemu: qmp query-cpu-model-expansion command qemu-caps: Get host model directly from Qemu when available Jason J. Herne (2): s390: Cpu driver support for getModels, update and compare s390: qemu: Support cpu host-model mode src/cpu/cpu_s390.c | 33 +++++++++++++-- src/qemu/qemu_capabilities.c | 88 ++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_command.c | 3 ++ src/qemu/qemu_monitor.c | 28 +++++++++++++ src/qemu/qemu_monitor.h | 19 +++++++++ src/qemu/qemu_monitor_json.c | 98 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 6 +++ 7 files changed, 271 insertions(+), 4 deletions(-) -- 1.9.1
Implement getModels for s390. It returns an empty list. This means libvirt supports all models Qemu knows about. Implement compare for s390. Required to test the guest against the host for guest cpu model runnability checking. We always return IDENTICAL to bypass Libvirt's checking. s390 will rely on Qemu to perform the runnability checking. Implement update for s390. required to support use of cpu "host-model" mode. Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> --- src/cpu/cpu_s390.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c index fb352a0..0f94084 100644 --- a/src/cpu/cpu_s390.c +++ b/src/cpu/cpu_s390.c @@ -71,16 +71,43 @@ s390DataFree(virCPUDataPtr data) VIR_FREE(data); } +static int +s390GetModels(char ***models ATTRIBUTE_UNUSED) +{ + return 0; +} + +static virCPUCompareResult +virCPUs390Compare(virCPUDefPtr host ATTRIBUTE_UNUSED, + virCPUDefPtr cpu ATTRIBUTE_UNUSED, + bool failMessages ATTRIBUTE_UNUSED) +{ + return VIR_CPU_COMPARE_IDENTICAL; +} + +static int +virCPUs390Update(virCPUDefPtr guest ATTRIBUTE_UNUSED, + const virCPUDef *host ATTRIBUTE_UNUSED) +{ + /* + * - host-passthrough not yet supported + * - host-model needs no changes + * - custom mode ... ??? + */ + return 0; +} + struct cpuArchDriver cpuDriverS390 = { .name = "s390", .arch = archs, .narch = ARRAY_CARDINALITY(archs), - .compare = NULL, + .compare = virCPUs390Compare, .decode = s390Decode, .encode = NULL, .free = s390DataFree, .nodeData = s390NodeData, .guestData = NULL, .baseline = NULL, - .update = NULL, + .update = virCPUs390Update, + .getModels = s390GetModels, }; -- 1.9.1
On Wed, Nov 02, 2016 at 16:34:31 -0400, Jason J. Herne wrote:
Implement getModels for s390. It returns an empty list. This means libvirt supports all models Qemu knows about.
Implement compare for s390. Required to test the guest against the host for guest cpu model runnability checking. We always return IDENTICAL to bypass Libvirt's checking. s390 will rely on Qemu to perform the runnability checking.
Implement update for s390. required to support use of cpu "host-model" mode.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> --- src/cpu/cpu_s390.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c index fb352a0..0f94084 100644 --- a/src/cpu/cpu_s390.c +++ b/src/cpu/cpu_s390.c @@ -71,16 +71,43 @@ s390DataFree(virCPUDataPtr data) VIR_FREE(data); }
+static int +s390GetModels(char ***models ATTRIBUTE_UNUSED) +{ + return 0; +}
This is almost an equivalent of not defining the function at all. Except that your code leaves models uninitialized. Keeping cpuArchDriver.getModels == NULL will do a better job (see cpu.c): if (!driver->getModels) { if (models) *models = NULL; return 0; }
+ +static virCPUCompareResult +virCPUs390Compare(virCPUDefPtr host ATTRIBUTE_UNUSED, + virCPUDefPtr cpu ATTRIBUTE_UNUSED, + bool failMessages ATTRIBUTE_UNUSED) +{ + return VIR_CPU_COMPARE_IDENTICAL; +} + +static int +virCPUs390Update(virCPUDefPtr guest ATTRIBUTE_UNUSED, + const virCPUDef *host ATTRIBUTE_UNUSED) +{ + /* + * - host-passthrough not yet supported
Why is it not supported?
+ * - host-model needs no changes
It actually needs changes. The CPU definition with mode='host-model' needs to be replaced with mode='custom' and model name and possibly features need to be set too.
+ * - custom mode ... ???
Custom mode would need to be changed only when match='minimum' is used. If that's not supported with s390, we should report an error. Hmm, which reminds me, we should probably report what matches are supported in domain capabilities. Jirka
From: "Collin L. Walling" <walling@linux.vnet.ibm.com> On s390 , the host's features are heavily influenced by not only the host hardware but also by hardware microcode level, host OS version, qemu version and kvm version. In this environment it does not make sense to attempt to report exact host details. Rather than use the generic "host" we leave this field blank. Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> --- src/cpu/cpu_s390.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c index 0f94084..c75eacb 100644 --- a/src/cpu/cpu_s390.c +++ b/src/cpu/cpu_s390.c @@ -59,7 +59,7 @@ s390Decode(virCPUDefPtr cpu, virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); if (cpu->model == NULL && - VIR_STRDUP(cpu->model, "host") < 0) + VIR_STRDUP(cpu->model, "") < 0) return -1; return 0; -- 1.9.1
On Wed, Nov 02, 2016 at 16:34:32 -0400, Jason J. Herne wrote:
From: "Collin L. Walling" <walling@linux.vnet.ibm.com>
On s390 , the host's features are heavily influenced by not only the host hardware but also by hardware microcode level, host OS version, qemu version and kvm version. In this environment it does not make sense to attempt to report exact host details. Rather than use the generic "host" we leave this field blank.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> --- src/cpu/cpu_s390.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c index 0f94084..c75eacb 100644 --- a/src/cpu/cpu_s390.c +++ b/src/cpu/cpu_s390.c @@ -59,7 +59,7 @@ s390Decode(virCPUDefPtr cpu, virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1);
if (cpu->model == NULL && - VIR_STRDUP(cpu->model, "host") < 0) + VIR_STRDUP(cpu->model, "") < 0) return -1;
return 0;
I think this function shouldn't do anything. Reporting "host" or even "" as host CPU is pointless. If we cannot provide anything reasonable, we should not report it at all. Jirka
On 11/03/2016 08:54 AM, Jiri Denemark wrote:
On Wed, Nov 02, 2016 at 16:34:32 -0400, Jason J. Herne wrote:
From: "Collin L. Walling" <walling@linux.vnet.ibm.com>
On s390 , the host's features are heavily influenced by not only the host hardware but also by hardware microcode level, host OS version, qemu version and kvm version. In this environment it does not make sense to attempt to report exact host details. Rather than use the generic "host" we leave this field blank.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> --- src/cpu/cpu_s390.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c index 0f94084..c75eacb 100644 --- a/src/cpu/cpu_s390.c +++ b/src/cpu/cpu_s390.c @@ -59,7 +59,7 @@ s390Decode(virCPUDefPtr cpu, virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1);
if (cpu->model == NULL && - VIR_STRDUP(cpu->model, "host") < 0) + VIR_STRDUP(cpu->model, "") < 0) return -1;
return 0;
I think this function shouldn't do anything. Reporting "host" or even "" as host CPU is pointless. If we cannot provide anything reasonable, we should not report it at all.
I would agree. But virsh domcapabilities only indicates support for host-model mode if we have something in cpu->hostModel. virDomainCapsCPUFormat() ... if (cpu->hostModel) { virBufferAddLit(buf, "supported='yes'>\n"); It also causes the guest to fail when trying to use host-model mode because virQEMUCapsInitHostCPUModel() skips setting qemuCaps->hostCPUModel if caps->host.cpu->model does not exist. Using an empty string here fixes both. Should I stick with it, or should we fix the problems elsewhere? -- -- Jason J. Herne (jjherne@linux.vnet.ibm.com)
On Fri, Nov 04, 2016 at 11:00:29 -0400, Jason J. Herne wrote:
On 11/03/2016 08:54 AM, Jiri Denemark wrote:
On Wed, Nov 02, 2016 at 16:34:32 -0400, Jason J. Herne wrote:
From: "Collin L. Walling" <walling@linux.vnet.ibm.com>
On s390 , the host's features are heavily influenced by not only the host hardware but also by hardware microcode level, host OS version, qemu version and kvm version. In this environment it does not make sense to attempt to report exact host details. Rather than use the generic "host" we leave this field blank.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> --- src/cpu/cpu_s390.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c index 0f94084..c75eacb 100644 --- a/src/cpu/cpu_s390.c +++ b/src/cpu/cpu_s390.c @@ -59,7 +59,7 @@ s390Decode(virCPUDefPtr cpu, virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1);
if (cpu->model == NULL && - VIR_STRDUP(cpu->model, "host") < 0) + VIR_STRDUP(cpu->model, "") < 0) return -1;
return 0;
I think this function shouldn't do anything. Reporting "host" or even "" as host CPU is pointless. If we cannot provide anything reasonable, we should not report it at all.
I would agree. But virsh domcapabilities only indicates support for host-model mode if we have something in cpu->hostModel.
virDomainCapsCPUFormat() ... if (cpu->hostModel) { virBufferAddLit(buf, "supported='yes'>\n");
It also causes the guest to fail when trying to use host-model mode because virQEMUCapsInitHostCPUModel() skips setting qemuCaps->hostCPUModel if caps->host.cpu->model does not exist.
And that's what you need to change. As I wrote in my comments to patch 4/5, virQEMUCapsInitHostCPUModel() is the place where qemuCaps->hostCPUModel should be initialized with the host CPU returned by QEMU. If you do that, all the issues you mentioned will go away. Jirka
From: "Collin L. Walling" <walling@linux.vnet.ibm.com> query-cpu-model-expansion is used to get a list of features for a given cpu model name or to get the model and features of the host hardware/environment as seen by Qemu/kvm. Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> --- src/qemu/qemu_monitor.c | 28 +++++++++++++ src/qemu/qemu_monitor.h | 19 +++++++++ src/qemu/qemu_monitor_json.c | 98 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 6 +++ 4 files changed, 151 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a5e14b2..a7f056b 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3615,6 +3615,34 @@ qemuMonitorCPUDefInfoFree(qemuMonitorCPUDefInfoPtr cpu) int +qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, + const char *type, + const char *model_name, + qemuMonitorCPUModelInfoPtr *model_info) +{ + VIR_DEBUG("model_info=%p", model_info); + + QEMU_CHECK_MONITOR_JSON(mon); + + return qemuMonitorJSONGetCPUModelExpansion(mon, type, model_name, model_info); +} + + +void +qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info) +{ + size_t i; + + if (!model_info) + return; + VIR_FREE(model_info->name); + for (i = 0; i < model_info->nprops; i++) + VIR_FREE(model_info->props[i].name); + VIR_FREE(model_info); +} + + +int qemuMonitorGetCommands(qemuMonitorPtr mon, char ***commands) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index c3133c4..60e72df 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -920,6 +920,25 @@ int qemuMonitorGetCPUDefinitions(qemuMonitorPtr mon, qemuMonitorCPUDefInfoPtr **cpus); void qemuMonitorCPUDefInfoFree(qemuMonitorCPUDefInfoPtr cpu); +typedef struct _qemuMonitorCPUModelInfo qemuMonitorCPUModelInfo; +typedef qemuMonitorCPUModelInfo *qemuMonitorCPUModelInfoPtr; + +struct _qemuMonitorCPUModelInfo { + char *name; + size_t nprops; + struct { + char *name; + bool supported; + } *props; +}; + +int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, + const char *type, + const char *model_name, + qemuMonitorCPUModelInfoPtr *model_info); + +void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info); + int qemuMonitorGetCommands(qemuMonitorPtr mon, char ***commands); int qemuMonitorGetEvents(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 6c13832..dbb92e1 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4975,6 +4975,104 @@ qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon, return ret; } +int +qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, + const char *type, + const char *model_name, + qemuMonitorCPUModelInfoPtr *model_info) +{ + int ret = -1; + virJSONValuePtr model; + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + virJSONValuePtr cpu_model; + virJSONValuePtr cpu_props; + qemuMonitorCPUModelInfoPtr newmodel = NULL; + char const *cpu_name; + int n; + size_t i; + + *model_info = NULL; + + if (!(model = virJSONValueNewObject())) + goto cleanup; + + if (virJSONValueObjectAppendString(model, "name", model_name) < 0) + goto cleanup; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-expansion", + "s:type", type, + "a:model", model, + NULL))) + goto cleanup; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONHasError(reply, "GenericError")) { + ret = 0; + goto cleanup; + } + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + if (!(data = virJSONValueObjectGetObject(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply was missing return data")); + goto cleanup; + } + + if (!(cpu_model = virJSONValueObjectGetObject(data, "model"))) + goto cleanup; + + if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) + goto cleanup; + + if (!(cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) + goto cleanup; + + if (VIR_ALLOC(newmodel) < 0) + goto cleanup; + + if (VIR_STRDUP(newmodel->name, cpu_name) < 0) + goto cleanup; + + n = newmodel->nprops = cpu_props->data.object.npairs; + + if (VIR_ALLOC_N(newmodel->props, n) < 0) + goto cleanup; + + for (i = 0; i < n; i++) { + const char *prop_name; + bool supported; + + if (!(prop_name = virJSONValueObjectGetKey(cpu_props, i))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply data is missing a property")); + goto cleanup; + } + if (VIR_STRDUP(newmodel->props[i].name, prop_name) < 0) + goto cleanup; + + if (virJSONValueObjectGetBoolean(cpu_props, prop_name, &supported) < 0) + goto cleanup; + newmodel->props[i].supported = supported; + } + + ret = 0; + *model_info = newmodel; + newmodel = NULL; + + cleanup: + qemuMonitorCPUModelInfoFree(newmodel); + virJSONValueFree(cmd); + virJSONValueFree(reply); + virJSONValueFree(model); + return ret; +} + int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, char ***commands) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 77b2e02..6e544c6 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -351,6 +351,12 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon, qemuMonitorCPUDefInfoPtr **cpus) ATTRIBUTE_NONNULL(2); +int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, + const char *type, + const char *model_name, + qemuMonitorCPUModelInfoPtr *model_info) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); + int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, char ***commands) ATTRIBUTE_NONNULL(2); -- 1.9.1
On Wed, Nov 02, 2016 at 16:34:33 -0400, Jason J. Herne wrote:
From: "Collin L. Walling" <walling@linux.vnet.ibm.com>
query-cpu-model-expansion is used to get a list of features for a given cpu model name or to get the model and features of the host hardware/environment as seen by Qemu/kvm.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> --- src/qemu/qemu_monitor.c | 28 +++++++++++++ src/qemu/qemu_monitor.h | 19 +++++++++ src/qemu/qemu_monitor_json.c | 98 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 6 +++ 4 files changed, 151 insertions(+)
I didn't look into details, but this patch seems fine. Jirka
From: "Collin L. Walling" <walling@linux.vnet.ibm.com> When qmp query-cpu-model-expansion is available probe Qemu for its view of the host model. In kvm environments this can provide a more complete view of the host model because features supported by Qemu and Kvm can be considered. Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 88 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 87 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 7a8202a..4a6ae07 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -389,6 +389,8 @@ struct _virQEMUCaps { size_t ngicCapabilities; virGICCapability *gicCapabilities; + virCPUDefPtr hostCPUModelFromQemu; + /* Anything below is not stored in the cache since the values are * re-computed from the other fields or external data sources every * time we probe QEMU or load the results from the cache. @@ -2118,6 +2120,10 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps) !(ret->hostCPUModel = virCPUDefCopy(qemuCaps->hostCPUModel))) goto error; + if (qemuCaps->hostCPUModelFromQemu && + !(ret->hostCPUModelFromQemu = virCPUDefCopy(qemuCaps->hostCPUModelFromQemu))) + goto error; + if (VIR_ALLOC_N(ret->machineTypes, qemuCaps->nmachineTypes) < 0) goto error; ret->nmachineTypes = qemuCaps->nmachineTypes; @@ -2728,6 +2734,51 @@ virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps, return ret; } +static int +virQEMUCapsProbeQMPCPUModelExpansion(virQEMUCapsPtr qemuCaps, + qemuMonitorPtr mon) +{ + qemuMonitorCPUModelInfoPtr model_info; + virCPUDefPtr hostcpumodel; + int nfeatures; + int ret = -1; + size_t i; + + if (qemuMonitorGetCPUModelExpansion(mon, "static", "host", &model_info) < 0) + goto cleanup; + + if (model_info == NULL) { + ret = 0; + goto cleanup; + } + + if (VIR_ALLOC(hostcpumodel) < 0) + goto cleanup; + + if (VIR_STRDUP(hostcpumodel->model, model_info->name) < 0) + goto cleanup; + + nfeatures = hostcpumodel->nfeatures = model_info->nprops; + + if (VIR_ALLOC_N(hostcpumodel->features, nfeatures) < 0) + goto cleanup; + + for (i = 0; i < nfeatures; i++) { + if (VIR_STRDUP(hostcpumodel->features[i].name, model_info->props[i].name) < 0) + goto cleanup; + + hostcpumodel->features[i].policy = -1; + } + + hostcpumodel->arch = qemuCaps->arch; + qemuCaps->hostCPUModelFromQemu = virCPUDefCopy(hostcpumodel); + ret = 0; + + cleanup: + virCPUDefFree(hostcpumodel); + return ret; +} + struct tpmTypeToCaps { int type; virQEMUCapsFlags caps; @@ -2958,6 +3009,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, virCapsPtr caps) { virCPUDefPtr cpu = NULL; + virCPUDefPtr src = NULL; if (!caps) return; @@ -2974,7 +3026,10 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, cpu->mode = VIR_CPU_MODE_CUSTOM; cpu->match = VIR_CPU_MATCH_EXACT; - if (virCPUDefCopyModelFilter(cpu, caps->host.cpu, true, + if (!(src = qemuCaps->hostCPUModelFromQemu)) + src = caps->host.cpu; + + if (virCPUDefCopyModelFilter(cpu, src, true, virQEMUCapsCPUFilterFeatures, NULL) < 0) goto error; } @@ -3118,6 +3173,21 @@ virQEMUCapsLoadCache(virCapsPtr caps, } VIR_FREE(str); + xmlNodePtr node; + if ((node = virXPathNode("./host/cpu[1]", ctxt))) { + xmlNodePtr oldNode = ctxt->node; + ctxt->node = node; + if (!(qemuCaps->hostCPUModelFromQemu = virCPUDefParseXML(node, + ctxt, + VIR_CPU_TYPE_HOST))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing host cpu data in QEMU capabilities cache")); + goto cleanup; + } + ctxt->node = oldNode; + node = NULL; + } + if ((n = virXPathNodeSet("./cpu", ctxt, &nodes)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to parse qemu capabilities cpus")); @@ -3298,6 +3368,20 @@ virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps, virBufferAsprintf(&buf, "<arch>%s</arch>\n", virArchToString(qemuCaps->arch)); + if (qemuCaps->hostCPUModelFromQemu) { + virBufferAddLit(&buf, "<host>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAddLit(&buf, "<cpu>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferEscapeString(&buf, "<arch>%s</arch>\n", + virArchToString(qemuCaps->hostCPUModelFromQemu->arch)); + virCPUDefFormatBuf(&buf, qemuCaps->hostCPUModelFromQemu, true); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</cpu>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</host>\n"); + } + if (qemuCaps->cpuDefinitions) { for (i = 0; i < qemuCaps->cpuDefinitions->nmodels; i++) { virDomainCapsCPUModelPtr cpu = qemuCaps->cpuDefinitions->models + i; @@ -3790,6 +3874,8 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, goto cleanup; if (virQEMUCapsProbeQMPCPUDefinitions(qemuCaps, mon) < 0) goto cleanup; + if (virQEMUCapsProbeQMPCPUModelExpansion(qemuCaps, mon) < 0) + goto cleanup; if (virQEMUCapsProbeQMPKVMState(qemuCaps, mon) < 0) goto cleanup; if (virQEMUCapsProbeQMPTPM(qemuCaps, mon) < 0) -- 1.9.1
On Wed, Nov 02, 2016 at 16:34:34 -0400, Jason J. Herne wrote:
From: "Collin L. Walling" <walling@linux.vnet.ibm.com>
When qmp query-cpu-model-expansion is available probe Qemu for its view of the host model. In kvm environments this can provide a more complete view of the host model because features supported by Qemu and Kvm can be considered.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 88 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 87 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 7a8202a..4a6ae07 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -389,6 +389,8 @@ struct _virQEMUCaps { size_t ngicCapabilities; virGICCapability *gicCapabilities;
+ virCPUDefPtr hostCPUModelFromQemu; +
Heh, why? We already have virCPUDefPtr hostCPUModel in this struct and it was added exactly for the purpose of storing the host CPU model as probed from QEMU. Only if probing QEMU cannot be used (which is the current state), we copy the host CPU from virCaps.
/* Anything below is not stored in the cache since the values are * re-computed from the other fields or external data sources every * time we probe QEMU or load the results from the cache. @@ -2118,6 +2120,10 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps) !(ret->hostCPUModel = virCPUDefCopy(qemuCaps->hostCPUModel))) goto error;
+ if (qemuCaps->hostCPUModelFromQemu && + !(ret->hostCPUModelFromQemu = virCPUDefCopy(qemuCaps->hostCPUModelFromQemu))) + goto error; + if (VIR_ALLOC_N(ret->machineTypes, qemuCaps->nmachineTypes) < 0) goto error; ret->nmachineTypes = qemuCaps->nmachineTypes; @@ -2728,6 +2734,51 @@ virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps, return ret; }
+static int +virQEMUCapsProbeQMPCPUModelExpansion(virQEMUCapsPtr qemuCaps, + qemuMonitorPtr mon) +{ + qemuMonitorCPUModelInfoPtr model_info; + virCPUDefPtr hostcpumodel; + int nfeatures; + int ret = -1; + size_t i; + + if (qemuMonitorGetCPUModelExpansion(mon, "static", "host", &model_info) < 0) + goto cleanup; + + if (model_info == NULL) { + ret = 0; + goto cleanup; + } + + if (VIR_ALLOC(hostcpumodel) < 0) + goto cleanup; + + if (VIR_STRDUP(hostcpumodel->model, model_info->name) < 0) + goto cleanup; + + nfeatures = hostcpumodel->nfeatures = model_info->nprops; + + if (VIR_ALLOC_N(hostcpumodel->features, nfeatures) < 0) + goto cleanup; + + for (i = 0; i < nfeatures; i++) { + if (VIR_STRDUP(hostcpumodel->features[i].name, model_info->props[i].name) < 0) + goto cleanup; + + hostcpumodel->features[i].policy = -1; + } + + hostcpumodel->arch = qemuCaps->arch; + qemuCaps->hostCPUModelFromQemu = virCPUDefCopy(hostcpumodel); + ret = 0; + + cleanup: + virCPUDefFree(hostcpumodel); + return ret; +}
The virQEMUCapsProbeQMPCPUModelExpansion (although I'd call it virQEMUCapsProbeQMPHostCPU) should just call qemuMonitorGetCPUModelExpansion and store the result in qemuCaps so that it can be stored in the capabilities cache. The rest of the code which takes the raw data from QEMU and creates host CPU model definition from it should go into virQEMUCapsInitHostCPUModel.
+ struct tpmTypeToCaps { int type; virQEMUCapsFlags caps; @@ -2958,6 +3009,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, virCapsPtr caps) { virCPUDefPtr cpu = NULL; + virCPUDefPtr src = NULL;
if (!caps) return; @@ -2974,7 +3026,10 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, cpu->mode = VIR_CPU_MODE_CUSTOM; cpu->match = VIR_CPU_MATCH_EXACT;
- if (virCPUDefCopyModelFilter(cpu, caps->host.cpu, true, + if (!(src = qemuCaps->hostCPUModelFromQemu)) + src = caps->host.cpu; + + if (virCPUDefCopyModelFilter(cpu, src, true, virQEMUCapsCPUFilterFeatures, NULL) < 0) goto error; } @@ -3118,6 +3173,21 @@ virQEMUCapsLoadCache(virCapsPtr caps, } VIR_FREE(str);
+ xmlNodePtr node; + if ((node = virXPathNode("./host/cpu[1]", ctxt))) { + xmlNodePtr oldNode = ctxt->node; + ctxt->node = node; + if (!(qemuCaps->hostCPUModelFromQemu = virCPUDefParseXML(node, + ctxt, + VIR_CPU_TYPE_HOST))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing host cpu data in QEMU capabilities cache")); + goto cleanup; + } + ctxt->node = oldNode; + node = NULL; + } +
As I said above, it's the reply from QEMU (qemuMonitorCPUModelInfoPtr) that should be loaded from the cache.
if ((n = virXPathNodeSet("./cpu", ctxt, &nodes)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to parse qemu capabilities cpus")); @@ -3298,6 +3368,20 @@ virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps, virBufferAsprintf(&buf, "<arch>%s</arch>\n", virArchToString(qemuCaps->arch));
+ if (qemuCaps->hostCPUModelFromQemu) { + virBufferAddLit(&buf, "<host>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAddLit(&buf, "<cpu>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferEscapeString(&buf, "<arch>%s</arch>\n", + virArchToString(qemuCaps->hostCPUModelFromQemu->arch)); + virCPUDefFormatBuf(&buf, qemuCaps->hostCPUModelFromQemu, true); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</cpu>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</host>\n"); + } +
And qemuMonitorCPUModelInfoPtr should be stored in the cache.
if (qemuCaps->cpuDefinitions) { for (i = 0; i < qemuCaps->cpuDefinitions->nmodels; i++) { virDomainCapsCPUModelPtr cpu = qemuCaps->cpuDefinitions->models + i; @@ -3790,6 +3874,8 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, goto cleanup; if (virQEMUCapsProbeQMPCPUDefinitions(qemuCaps, mon) < 0) goto cleanup; + if (virQEMUCapsProbeQMPCPUModelExpansion(qemuCaps, mon) < 0) + goto cleanup; if (virQEMUCapsProbeQMPKVMState(qemuCaps, mon) < 0) goto cleanup; if (virQEMUCapsProbeQMPTPM(qemuCaps, mon) < 0)
Jirka
Look up the host model stored in qemuCaps for generation of the -cpu model portion of the Qemu command line. Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b68da3d..3b7095a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6537,6 +6537,9 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver, virBufferAddLit(buf, "host"); if (cpu->model) virBufferAsprintf(buf, ",compat=%s", cpu->model); + } else if (ARCH_IS_S390(def->os.arch)) { + virCPUDefPtr guestCpu = virQEMUCapsGetHostModel(qemuCaps); + virBufferAdd(buf, guestCpu->model, -1); } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected host-model CPU for %s architecture"), -- 1.9.1
On Wed, Nov 02, 2016 at 16:34:35 -0400, Jason J. Herne wrote:
Look up the host model stored in qemuCaps for generation of the -cpu model portion of the Qemu command line.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b68da3d..3b7095a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6537,6 +6537,9 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver, virBufferAddLit(buf, "host"); if (cpu->model) virBufferAsprintf(buf, ",compat=%s", cpu->model); + } else if (ARCH_IS_S390(def->os.arch)) { + virCPUDefPtr guestCpu = virQEMUCapsGetHostModel(qemuCaps); + virBufferAdd(buf, guestCpu->model, -1); } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected host-model CPU for %s architecture"),
No, there's no need to change anything in qemuBuildCpuModelArgStr. virCPUUpdate should have translated host-model CPU definition into the real CPU model (the one from virQEMUCapsGetHostModel(qemuCaps)). Jirka
On Wed, Nov 02, 2016 at 16:34:30 -0400, Jason J. Herne wrote: ...
Patch #5 updates qemuBuildCpuModelArgStr's VIR_CPU_MODE_HOST_MODEL case. It seems like all archs except PPC64 do not support host model mode.
x86 supports host-model too, but ppc64 (mis)used the host-model mode in a very specific way.
Here is a list of outstanding things to do and some questions.
1. virsh domcapabilities --emulatorbin flag
This works today if the command is passed --emulatorbin /usr/bin/qemu-kvm. If no emulatorbin is given then Libvirt seems to choose /usr/bin/qemu-system-s390x my system which does not have kvm enabled.
Does s390x still use the abandoned qemu-kvm name or is it just the stupid shell wrapper which calls the real binary with -machine accel=kvm? You say /usr/bin/qemu-system-s390x does not have kvm enabled, does it mean you cannot use kvm with this binary at all or you need to explicitly enable it with the -machine accel=kvm option?
A kvm session is needed for s390 to properly compute the host model. A kvm session is only available for the qemu-kvm binary.
Right, and the series I sent for review yesterday changes our probing code to use kvm if it is available.
Is the answer to tell our users to always supply --emulatorbin with /usr/bin/qemu-kvm argument?
No it shouldn't be necessary, --emulatorbin is needed only if you want to use a QEMU binary which libvirt does not know about.
2. virsh baseline and compare support.
Both of these commands, for s390 at least, will need to spin up a Qemu session with monitor to compute the result. The only place I see this done today is qemu_capabilities. Rather than blindly duplicate the code, I guess we should carve out some type of api for spinning up a monitor backed Qemu instance, yes?
Also, neither compare nor baseline have the --emulatorbin flag. So we'll either need to add them or find an alternate solution. Any thoughts on this?
Yes, we will probably need to add new APIs which would accept more parameters, but I'd keep it for later.
3.Support host passthrough mode.
This essentially just means passing -cpu host to qemu. We know Qemu supports this for s390 today. But Libvirt claims it is not supported due to the following:
In virQEMUCapsIsCPUModeSupported, our virt "type" is VIR_DOMAIN_VIRT_QEMU, and not VIR_DOMAIN_VIRT_KVM.
Well, this should be correct, or is QEMU on s390 able to use -cpu host in TCG mode? Does "virsh domcapabilities --virttype kvm" report host-passthrough as supported?
virttype comes from domCaps and is set here: qemuConnectGetDomainCapabilities() The relevant code is:
if (qemuHostdevHostSupportsPassthroughLegacy() || qemuHostdevHostSupportsPassthroughVFIO()) virttype = VIR_DOMAIN_VIRT_KVM; else virttype = VIR_DOMAIN_VIRT_QEMU;
I'll admit, I have not been able to figure out why these checks are failing in my test environment, when I suspect they should be passing. But in my case it seems as though these relate to host device passthrough. How do they apply to cpu model passthrough. What am I missing?
They are not related at all. The code is just setting a default value for virttype if the caller doesn't set it explicitly. The logic looks a bit weird, but it doesn't really matter, since you can ask for the virttype you need explicitly. Jirka
On 11/03/2016 02:37 PM, Jiri Denemark wrote:
On Wed, Nov 02, 2016 at 16:34:30 -0400, Jason J. Herne wrote: ...
Patch #5 updates qemuBuildCpuModelArgStr's VIR_CPU_MODE_HOST_MODEL case. It seems like all archs except PPC64 do not support host model mode.
x86 supports host-model too, but ppc64 (mis)used the host-model mode in a very specific way.
Here is a list of outstanding things to do and some questions.
1. virsh domcapabilities --emulatorbin flag
This works today if the command is passed --emulatorbin /usr/bin/qemu-kvm. If no emulatorbin is given then Libvirt seems to choose /usr/bin/qemu-system-s390x my system which does not have kvm enabled.
Does s390x still use the abandoned qemu-kvm name or is it just the stupid shell wrapper which calls the real binary with -machine accel=kvm? You say /usr/bin/qemu-system-s390x does not have kvm enabled, does it mean you cannot use kvm with this binary at all or you need to explicitly enable it with the -machine accel=kvm option?
Its the latter. qemu-system-s390x has tcg and kvm and requires accel=kvm to return useful data
On Thu, Nov 03, 2016 at 14:39:39 +0100, Christian Borntraeger wrote:
On 11/03/2016 02:37 PM, Jiri Denemark wrote:
On Wed, Nov 02, 2016 at 16:34:30 -0400, Jason J. Herne wrote: ...
Patch #5 updates qemuBuildCpuModelArgStr's VIR_CPU_MODE_HOST_MODEL case. It seems like all archs except PPC64 do not support host model mode.
x86 supports host-model too, but ppc64 (mis)used the host-model mode in a very specific way.
Here is a list of outstanding things to do and some questions.
1. virsh domcapabilities --emulatorbin flag
This works today if the command is passed --emulatorbin /usr/bin/qemu-kvm. If no emulatorbin is given then Libvirt seems to choose /usr/bin/qemu-system-s390x my system which does not have kvm enabled.
Does s390x still use the abandoned qemu-kvm name or is it just the stupid shell wrapper which calls the real binary with -machine accel=kvm? You say /usr/bin/qemu-system-s390x does not have kvm enabled, does it mean you cannot use kvm with this binary at all or you need to explicitly enable it with the -machine accel=kvm option?
Its the latter. qemu-system-s390x has tcg and kvm and requires accel=kvm to return useful data
Good, so it's the same as with x86. This issue should be fixed by "qemu: Add support for unavailable-features" series. Jirka
On 11/03/2016 02:37 PM, Jiri Denemark wrote:
Well, this should be correct, or is QEMU on s390 able to use -cpu host in TCG mode?
Does "virsh domcapabilities --virttype kvm" report host-passthrough as supported?
yes, this is what I get <domainCapabilities> <path>/usr/bin/qemu-system-s390x</path> <domain>kvm</domain> <machine>s390-ccw-virtio-2.8</machine> <arch>s390x</arch> <vcpu max='248'/> <os supported='yes'> <loader supported='yes'> <enum name='type'> <value>rom</value> <value>pflash</value> </enum> <enum name='readonly'> <value>yes</value> <value>no</value> </enum> </loader> </os> <cpu> <mode name='host-passthrough' supported='yes'/> <mode name='host-model' supported='yes'> <model fallback='allow'>host</model> </mode> <mode name='custom' supported='yes'> <model usable='unknown'>z10EC</model> <model usable='unknown'>z13s</model> <model usable='unknown'>z13</model> <model usable='unknown'>zEC12-base</model> <model usable='unknown'>z890.3</model> <model usable='unknown'>z890.2</model> <model usable='unknown'>z196.2-base</model> <model usable='unknown'>z890</model> <model usable='unknown'>z990-base</model> <model usable='unknown'>z900.2-base</model> <model usable='unknown'>z114</model> <model usable='unknown'>z13s-base</model> <model usable='unknown'>z196-base</model> <model usable='unknown'>z900</model> <model usable='unknown'>z9BC</model> <model usable='unknown'>z990.5-base</model> <model usable='unknown'>zEC12.2</model> <model usable='unknown'>zEC12.2-base</model> <model usable='unknown'>zBC12-base</model> <model usable='unknown'>z990</model> <model usable='unknown'>z10BC-base</model> <model usable='unknown'>z196.2</model> <model usable='unknown'>z890.3-base</model> <model usable='unknown'>z13.2</model> <model usable='unknown'>z10EC.3</model> <model usable='unknown'>z10EC.2</model> <model usable='unknown'>z10BC.2</model> <model usable='unknown'>host</model> <model usable='unknown'>z9EC.3-base</model> <model usable='unknown'>zEC12</model> <model usable='unknown'>zBC12</model> <model usable='unknown'>z10EC.3-base</model> <model usable='unknown'>qemu</model> <model usable='unknown'>z900.3-base</model> <model usable='unknown'>z9BC-base</model> <model usable='unknown'>z800-base</model> <model usable='unknown'>z890-base</model> <model usable='unknown'>z800</model> <model usable='unknown'>z990.5</model> <model usable='unknown'>z990.4</model> <model usable='unknown'>z9EC.2-base</model> <model usable='unknown'>z990.3</model> <model usable='unknown'>z990.2</model> <model usable='unknown'>z900.3</model> <model usable='unknown'>z900.2</model> <model usable='unknown'>z10EC-base</model> <model usable='unknown'>z10EC.2-base</model> <model usable='unknown'>z990.3-base</model> <model usable='unknown'>z900-base</model> <model usable='unknown'>z9BC.2-base</model> <model usable='unknown'>z114-base</model> <model usable='unknown'>z10BC.2-base</model> <model usable='unknown'>z13.2-base</model> <model usable='unknown'>z990.2-base</model> <model usable='unknown'>z9EC-base</model> <model usable='unknown'>z10BC</model> <model usable='unknown'>z9EC</model> <model usable='unknown'>z9EC.3</model> <model usable='unknown'>z9EC.2</model> <model usable='unknown'>z9BC.2</model> <model usable='unknown'>z13-base</model> <model usable='unknown'>z990.4-base</model> <model usable='unknown'>z890.2-base</model> <model usable='unknown'>z196</model> </mode> </cpu> <devices> <disk supported='yes'> <enum name='diskDevice'> <value>disk</value> <value>cdrom</value> <value>floppy</value> <value>lun</value> </enum> <enum name='bus'> <value>ide</value> <value>fdc</value> <value>scsi</value> <value>virtio</value> </enum> </disk> <graphics supported='yes'> <enum name='type'> <value>sdl</value> <value>vnc</value> </enum> </graphics> <video supported='yes'> <enum name='modelType'> <value>qxl</value> <value>virtio</value> </enum> </video> <hostdev supported='yes'> <enum name='mode'> <value>subsystem</value> </enum> <enum name='startupPolicy'> <value>default</value> <value>mandatory</value> <value>requisite</value> <value>optional</value> </enum> <enum name='subsysType'> <value>usb</value> <value>pci</value> <value>scsi</value> </enum> <enum name='capsType'/> <enum name='pciBackend'/> </hostdev> </devices> <features> <gic supported='no'/> </features> </domainCapabilities>
On Thu, Nov 03, 2016 at 14:41:25 +0100, Christian Borntraeger wrote:
On 11/03/2016 02:37 PM, Jiri Denemark wrote:
Well, this should be correct, or is QEMU on s390 able to use -cpu host in TCG mode?
Does "virsh domcapabilities --virttype kvm" report host-passthrough as supported?
yes, this is what I get
<domainCapabilities> ... <cpu> <mode name='host-passthrough' supported='yes'/>
Nice. So it seems to work as expected. Later we may look into the reasons why qemu virttype is returned by default. I find it pretty strange too since the defualt virttype should be the best one, i.e., kvm, but perhaps there were reasons for returning qemu type. Jirka
On 11/03/2016 09:37 AM, Jiri Denemark wrote:
On Wed, Nov 02, 2016 at 16:34:30 -0400, Jason J. Herne wrote: ...
2. virsh baseline and compare support.
Both of these commands, for s390 at least, will need to spin up a Qemu session with monitor to compute the result. The only place I see this done today is qemu_capabilities. Rather than blindly duplicate the code, I guess we should carve out some type of api for spinning up a monitor backed Qemu instance, yes?
...
Yes, we will probably need to add new APIs which would accept more parameters, but I'd keep it for later.
Thanks for looking this series over. We'll get to work on making the changes you suggest. Just one thing here. I'm not sure if you are saying to leave baseline/compare for later. Or do you mean we should hold off on the api that can be used by any qemu code to spawn a helper qemu process? If we are implementing baseline and compare then we'll need some way to invoke Qemu. -- -- Jason J. Herne (jjherne@linux.vnet.ibm.com)
On Thu, Nov 03, 2016 at 10:19:23 -0400, Jason J. Herne wrote:
On 11/03/2016 09:37 AM, Jiri Denemark wrote:
On Wed, Nov 02, 2016 at 16:34:30 -0400, Jason J. Herne wrote: ...
2. virsh baseline and compare support.
Both of these commands, for s390 at least, will need to spin up a Qemu session with monitor to compute the result. The only place I see this done today is qemu_capabilities. Rather than blindly duplicate the code, I guess we should carve out some type of api for spinning up a monitor backed Qemu instance, yes?
...
Yes, we will probably need to add new APIs which would accept more parameters, but I'd keep it for later.
Thanks for looking this series over. We'll get to work on making the changes you suggest. Just one thing here.
I'm not sure if you are saying to leave baseline/compare for later. Or do you mean we should hold off on the api that can be used by any qemu code to spawn a helper qemu process?
If we are implementing baseline and compare then we'll need some way to invoke Qemu.
Sure, I think we can just leave them both unimplemented for s390 now. Jirka
participants (3)
-
Christian Borntraeger -
Jason J. Herne -
Jiri Denemark