[libvirt] [PATCH v1] RFC Generating Qemu parameter -cpu features

I'm working on the CPU model checking support in libvirt for s390x and I've found when generating the -cpu +feature,-feature command, Qemu does not like the ± syntax when adding cpu features for s390x machines and expects us to use key=value pairs (e.g. feature=on,feature=off). Since x86 Qemu code supports both the ± and key=value methods of enabling CPU features, could we switch to just using key=value syntax in libvirt or do we need to maintain the ± format as well? I assume we could do something more sophisticated than wrapping the check in an if cpu->arch == VIR_ARCH_S390X? Another option would be to just change Qemu to support both formats, but I suspect we would get push back as key=value seems to be the preferred interface. Thanks for your time. - Collin Collin L. Walling (1): qemu: command: rework cpu feature argument support src/qemu/qemu_command.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.7.4

cpu features are passed to the qemu command with feature=on/off instead of +/-feature. Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b68da3d..a0eba0e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6560,12 +6560,12 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver, switch ((virCPUFeaturePolicy) cpu->features[i].policy) { case VIR_CPU_FEATURE_FORCE: case VIR_CPU_FEATURE_REQUIRE: - virBufferAsprintf(buf, ",+%s", cpu->features[i].name); + virBufferAsprintf(buf, ",%s=on", cpu->features[i].name); break; case VIR_CPU_FEATURE_DISABLE: case VIR_CPU_FEATURE_FORBID: - virBufferAsprintf(buf, ",-%s", cpu->features[i].name); + virBufferAsprintf(buf, ",%s=off", cpu->features[i].name); break; case VIR_CPU_FEATURE_OPTIONAL: -- 2.7.4

On Mon, Nov 14, 2016 at 02:26:03PM -0500, Collin L. Walling wrote:
cpu features are passed to the qemu command with feature=on/off instead of +/-feature.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
If I'm not mistaken, the "feature=on|off" syntax was added on QEMU 2.0.0. Does current libvirt support older QEMU versions?
--- src/qemu/qemu_command.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b68da3d..a0eba0e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6560,12 +6560,12 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver, switch ((virCPUFeaturePolicy) cpu->features[i].policy) { case VIR_CPU_FEATURE_FORCE: case VIR_CPU_FEATURE_REQUIRE: - virBufferAsprintf(buf, ",+%s", cpu->features[i].name); + virBufferAsprintf(buf, ",%s=on", cpu->features[i].name); break;
case VIR_CPU_FEATURE_DISABLE: case VIR_CPU_FEATURE_FORBID: - virBufferAsprintf(buf, ",-%s", cpu->features[i].name); + virBufferAsprintf(buf, ",%s=off", cpu->features[i].name); break;
case VIR_CPU_FEATURE_OPTIONAL: -- 2.7.4
-- Eduardo

On Mon, Nov 14, 2016 at 18:02:29 -0200, Eduardo Habkost wrote:
On Mon, Nov 14, 2016 at 02:26:03PM -0500, Collin L. Walling wrote:
cpu features are passed to the qemu command with feature=on/off instead of +/-feature.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
If I'm not mistaken, the "feature=on|off" syntax was added on QEMU 2.0.0. Does current libvirt support older QEMU versions?
Of course it does. I'd love to switch to feature=on|off, but how can we check if QEMU supports it? We can't really start using this syntax without it. Jirka

CCing qemu-devel. CCing Markus, in case he has any insights about the interface introspection. On Tue, Nov 15, 2016 at 08:42:12AM +0100, Jiri Denemark wrote:
On Mon, Nov 14, 2016 at 18:02:29 -0200, Eduardo Habkost wrote:
On Mon, Nov 14, 2016 at 02:26:03PM -0500, Collin L. Walling wrote:
cpu features are passed to the qemu command with feature=on/off instead of +/-feature.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
If I'm not mistaken, the "feature=on|off" syntax was added on QEMU 2.0.0. Does current libvirt support older QEMU versions?
Of course it does. I'd love to switch to feature=on|off, but how can we check if QEMU supports it? We can't really start using this syntax without it.
Actually, I was wrong, this was added in v2.4.0. "feat=on|off" needs two things to work (in x86): * Translation of all "foo=bar" options to QOM property setting. This was added in v2.0.0-rc0~162^2 * The actual QOM properties for feature names to be present. They were added in v2.4.0-rc0~101^2~1 So you can be sure "feat=on" is supported by checking if the feature flags are present in device-list-properties output for the CPU model. But device-list-properties is also messy[1]. Maybe we can use the availability of query-cpu-model-expansion to check if we can safely use the new "feat=on|off" system? It's easier than taking all the variables above into account. --- [1] * device-list-properties support for x86 CPU QOM classes will be in QEMU 2.8. * device-list-properties on x86 CPU QOM classes returns an error on QEMU 2.5-2.7. * device-list-properties on x86 CPU classes may crash QEMU in QEMU older than 2.5 (see commit 4c315c27). But: * query-cpu-definitions will probably return the CPU QOM typename in QEMU 2.9+ only. -- Eduardo

On Tue, Nov 15, 2016 at 11:44:00 -0200, Eduardo Habkost wrote:
CCing qemu-devel.
CCing Markus, in case he has any insights about the interface introspection.
On Tue, Nov 15, 2016 at 08:42:12AM +0100, Jiri Denemark wrote:
On Mon, Nov 14, 2016 at 18:02:29 -0200, Eduardo Habkost wrote:
On Mon, Nov 14, 2016 at 02:26:03PM -0500, Collin L. Walling wrote:
cpu features are passed to the qemu command with feature=on/off instead of +/-feature.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
If I'm not mistaken, the "feature=on|off" syntax was added on QEMU 2.0.0. Does current libvirt support older QEMU versions?
Of course it does. I'd love to switch to feature=on|off, but how can we check if QEMU supports it? We can't really start using this syntax without it.
Actually, I was wrong, this was added in v2.4.0. "feat=on|off" needs two things to work (in x86):
* Translation of all "foo=bar" options to QOM property setting. This was added in v2.0.0-rc0~162^2 * The actual QOM properties for feature names to be present. They were added in v2.4.0-rc0~101^2~1
So you can be sure "feat=on" is supported by checking if the feature flags are present in device-list-properties output for the CPU model. But device-list-properties is also messy[1].
Maybe we can use the availability of query-cpu-model-expansion to check if we can safely use the new "feat=on|off" system? It's easier than taking all the variables above into account.
Yeah, this could work since s390 already supports query-cpu-model-expansion. It would cause feature=on|off not to be used on x86_64 with QEMU older than 2.9.0, but I guess that's not a big deal, is it? Jirka

On Wed, Nov 16, 2016 at 02:15:02PM +0100, Jiri Denemark wrote:
On Tue, Nov 15, 2016 at 11:44:00 -0200, Eduardo Habkost wrote:
CCing qemu-devel.
CCing Markus, in case he has any insights about the interface introspection.
On Tue, Nov 15, 2016 at 08:42:12AM +0100, Jiri Denemark wrote:
On Mon, Nov 14, 2016 at 18:02:29 -0200, Eduardo Habkost wrote:
On Mon, Nov 14, 2016 at 02:26:03PM -0500, Collin L. Walling wrote:
cpu features are passed to the qemu command with feature=on/off instead of +/-feature.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
If I'm not mistaken, the "feature=on|off" syntax was added on QEMU 2.0.0. Does current libvirt support older QEMU versions?
Of course it does. I'd love to switch to feature=on|off, but how can we check if QEMU supports it? We can't really start using this syntax without it.
Actually, I was wrong, this was added in v2.4.0. "feat=on|off" needs two things to work (in x86):
* Translation of all "foo=bar" options to QOM property setting. This was added in v2.0.0-rc0~162^2 * The actual QOM properties for feature names to be present. They were added in v2.4.0-rc0~101^2~1
So you can be sure "feat=on" is supported by checking if the feature flags are present in device-list-properties output for the CPU model. But device-list-properties is also messy[1].
Maybe we can use the availability of query-cpu-model-expansion to check if we can safely use the new "feat=on|off" system? It's easier than taking all the variables above into account.
Yeah, this could work since s390 already supports query-cpu-model-expansion. It would cause feature=on|off not to be used on x86_64 with QEMU older than 2.9.0, but I guess that's not a big deal, is it?
Not a problem, as we have no plans to remove +feat/-feat support in x86 anymore. -- Eduardo

On 11/16/2016 09:05 AM, Eduardo Habkost wrote:
On Wed, Nov 16, 2016 at 02:15:02PM +0100, Jiri Denemark wrote:
On Tue, Nov 15, 2016 at 11:44:00 -0200, Eduardo Habkost wrote:
CCing qemu-devel.
CCing Markus, in case he has any insights about the interface introspection.
On Tue, Nov 15, 2016 at 08:42:12AM +0100, Jiri Denemark wrote:
On Mon, Nov 14, 2016 at 18:02:29 -0200, Eduardo Habkost wrote:
On Mon, Nov 14, 2016 at 02:26:03PM -0500, Collin L. Walling wrote:
cpu features are passed to the qemu command with feature=on/off instead of +/-feature.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> If I'm not mistaken, the "feature=on|off" syntax was added on QEMU 2.0.0. Does current libvirt support older QEMU versions? Of course it does. I'd love to switch to feature=on|off, but how can we check if QEMU supports it? We can't really start using this syntax without it. Actually, I was wrong, this was added in v2.4.0. "feat=on|off" needs two things to work (in x86):
* Translation of all "foo=bar" options to QOM property setting. This was added in v2.0.0-rc0~162^2 * The actual QOM properties for feature names to be present. They were added in v2.4.0-rc0~101^2~1
So you can be sure "feat=on" is supported by checking if the feature flags are present in device-list-properties output for the CPU model. But device-list-properties is also messy[1].
Maybe we can use the availability of query-cpu-model-expansion to check if we can safely use the new "feat=on|off" system? It's easier than taking all the variables above into account. Yeah, this could work since s390 already supports query-cpu-model-expansion. It would cause feature=on|off not to be used on x86_64 with QEMU older than 2.9.0, but I guess that's not a big deal, is it? Not a problem, as we have no plans to remove +feat/-feat support in x86 anymore.
Beautiful. Thanks for your responses everyone. :)

Eduardo Habkost <ehabkost@redhat.com> writes:
CCing qemu-devel.
CCing Markus, in case he has any insights about the interface introspection.
On Tue, Nov 15, 2016 at 08:42:12AM +0100, Jiri Denemark wrote:
On Mon, Nov 14, 2016 at 18:02:29 -0200, Eduardo Habkost wrote:
On Mon, Nov 14, 2016 at 02:26:03PM -0500, Collin L. Walling wrote:
cpu features are passed to the qemu command with feature=on/off instead of +/-feature.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
If I'm not mistaken, the "feature=on|off" syntax was added on QEMU 2.0.0. Does current libvirt support older QEMU versions?
Of course it does. I'd love to switch to feature=on|off, but how can we check if QEMU supports it? We can't really start using this syntax without it.
Actually, I was wrong, this was added in v2.4.0. "feat=on|off" needs two things to work (in x86):
* Translation of all "foo=bar" options to QOM property setting. This was added in v2.0.0-rc0~162^2 * The actual QOM properties for feature names to be present. They were added in v2.4.0-rc0~101^2~1
So you can be sure "feat=on" is supported by checking if the feature flags are present in device-list-properties output for the CPU model. But device-list-properties is also messy[1].
Maybe we can use the availability of query-cpu-model-expansion to check if we can safely use the new "feat=on|off" system? It's easier than taking all the variables above into account.
---
[1] * device-list-properties support for x86 CPU QOM classes will be in QEMU 2.8. * device-list-properties on x86 CPU QOM classes returns an error on QEMU 2.5-2.7. * device-list-properties on x86 CPU classes may crash QEMU in QEMU older than 2.5 (see commit 4c315c27). But: * query-cpu-definitions will probably return the CPU QOM typename in QEMU 2.9+ only.
In other words: * feature=on|off works since 2.4, but what's the best way to probe for it? * device-list-properties can act as a witness, but there are two problems: you need to map between CPU model and QOM typename to use it, and it can fail or even crash before 2.8. * 2.9 will take provide the information to map between CPU model and QOM typename. I guess the sane choice is to probe for the CPU model - QOM typename mapping information, and if it's there, assume device-list-properties and feature=on|off work. Eduardo, what do you think?

On Thu, Nov 17, 2016 at 04:04:33PM +0100, Markus Armbruster wrote:
Eduardo Habkost <ehabkost@redhat.com> writes:
CCing qemu-devel.
CCing Markus, in case he has any insights about the interface introspection.
On Tue, Nov 15, 2016 at 08:42:12AM +0100, Jiri Denemark wrote:
On Mon, Nov 14, 2016 at 18:02:29 -0200, Eduardo Habkost wrote:
On Mon, Nov 14, 2016 at 02:26:03PM -0500, Collin L. Walling wrote:
cpu features are passed to the qemu command with feature=on/off instead of +/-feature.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
If I'm not mistaken, the "feature=on|off" syntax was added on QEMU 2.0.0. Does current libvirt support older QEMU versions?
Of course it does. I'd love to switch to feature=on|off, but how can we check if QEMU supports it? We can't really start using this syntax without it.
Actually, I was wrong, this was added in v2.4.0. "feat=on|off" needs two things to work (in x86):
* Translation of all "foo=bar" options to QOM property setting. This was added in v2.0.0-rc0~162^2 * The actual QOM properties for feature names to be present. They were added in v2.4.0-rc0~101^2~1
So you can be sure "feat=on" is supported by checking if the feature flags are present in device-list-properties output for the CPU model. But device-list-properties is also messy[1].
Maybe we can use the availability of query-cpu-model-expansion to check if we can safely use the new "feat=on|off" system? It's easier than taking all the variables above into account.
---
[1] * device-list-properties support for x86 CPU QOM classes will be in QEMU 2.8. * device-list-properties on x86 CPU QOM classes returns an error on QEMU 2.5-2.7. * device-list-properties on x86 CPU classes may crash QEMU in QEMU older than 2.5 (see commit 4c315c27). But: * query-cpu-definitions will probably return the CPU QOM typename in QEMU 2.9+ only.
In other words:
* feature=on|off works since 2.4, but what's the best way to probe for it?
* device-list-properties can act as a witness, but there are two problems: you need to map between CPU model and QOM typename to use it, and it can fail or even crash before 2.8.
* 2.9 will take provide the information to map between CPU model and QOM typename.
I guess the sane choice is to probe for the CPU model - QOM typename mapping information, and if it's there, assume device-list-properties and feature=on|off work. Eduardo, what do you think?
That would work for x86, but not for s390x on QEMU <= 2.8 (where only feat=on|off is supported). Using query-cpu-model-expansion as a witness would work for both s390x and i386, even with QEMU <= 2.8. -- Eduardo
participants (4)
-
Collin L. Walling
-
Eduardo Habkost
-
Jiri Denemark
-
Markus Armbruster