On Wed, Sep 30, 2015 at 02:04:10PM +0300, Pavel Fedin wrote:
Support for GICv3 has been recently introduced in qemu using gic-version
option for the 'virt' machine. The option can actually take values of
'2', '3' and 'host', however, since in libvirt this is a numeric
parameter, we limit it only to 2 and 3. Value of 2 is not added to the
command line in order to keep backward compatibility with older qemu
versions.
Signed-off-by: Pavel Fedin <p.fedin@xxxxxxxxxxx>
---
src/qemu/qemu_command.c | 43 ++++++++++++++++++++++++++++++-------------
1 file changed, 30 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index bb1835c..a47e188 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7702,19 +7702,6 @@ qemuBuildCpuArgStr(virQEMUDriverPtr driver,
have_cpu = true;
}
- if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON) {
- if (def->gic_version && def->gic_version != 2) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("gic version '%u' is not supported"),
- def->gic_version);
- goto cleanup;
- }
-
- /* There's no command line argument currently to turn on/off GIC. It's
- * done automatically by qemu-system-aarch64. But if this changes, lets
- * put the code here. */
- }
-
if (virBufferCheckError(&buf) < 0)
goto cleanup;
@@ -7931,6 +7918,36 @@ qemuBuildMachineArgStr(virCommandPtr cmd,
return -1;
}
+ if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON) {
+ if (def->gic_version) {
+ if (!STREQ(def->os.machine, "virt") &&
+ !STRPREFIX(def->os.machine, "virt-")) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("gic-version option is available "
+ "only for virt machine"));
+ virBufferFreeAndReset(&buf);
+ return -1;
+ }
Indentation's off here. Also before this patch we would allow def->gic_version == 2 for any machine type. I don't have a problem with this since GIC doesn't make sense anywhere else then on ARM machines, but shouldn't we check for the fact that the request is for ARM (in case, for example, if ppc64 gains some 'virt' machine type)? Because we have no guarantee that it's ARM just based on the machine type.
+
+ /* 2 is the default, so we don't put it as option for
+ * backwards compatibility
+ */
+ if (def->gic_version != 2) {
+ if (virQEMUCapsGet(qemuCaps,
+ QEMU_CAPS_MACH_VIRT_GIC_VERSION)) {
+ virBufferAsprintf(&buf, ",gic-version=%d",
+ def->gic_version);
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("gic-version option is not available "
+ "with this QEMU binary"));
+ virBufferFreeAndReset(&buf);
+ return -1;
I'd change this to:
if (gic != 2) {
if (!caps)
error;
append_cmd();
}
If you're ok with that, just let me know and I'll push it with the
following diff squashed in, right after the release:
diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c
index a47e1883d976..72149bdd1eef 100644
--- i/src/qemu/qemu_command.c
+++ w/src/qemu/qemu_command.c
@@ -7918,35 +7918,36 @@ qemuBuildMachineArgStr(virCommandPtr cmd,
return -1;
}
- if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON) {
- if (def->gic_version) {
- if (!STREQ(def->os.machine, "virt") &&
- !STRPREFIX(def->os.machine, "virt-")) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON) {
+ if (def->gic_version) {
+ if ((def->os.arch != VIR_ARCH_ARMV7L &&
+ def->os.arch != VIR_ARCH_AARCH64) ||
+ (STRNEQ(def->os.machine, "virt") &&
+ !STRPREFIX(def->os.machine, "virt-"))) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("gic-version option is available "
"only for virt machine"));
virBufferFreeAndReset(&buf);
return -1;
- }
+ }
- /* 2 is the default, so we don't put it as option for
- * backwards compatibility
- */
+ /* 2 is the default, so we don't put it as option for
+ * backwards compatibility
+ */
if (def->gic_version != 2) {
- if (virQEMUCapsGet(qemuCaps,
- QEMU_CAPS_MACH_VIRT_GIC_VERSION)) {
- virBufferAsprintf(&buf, ",gic-version=%d",
- def->gic_version);
- } else {
+ if (!virQEMUCapsGet(qemuCaps,
+ QEMU_CAPS_MACH_VIRT_GIC_VERSION)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("gic-version option is not available "
"with this QEMU binary"));
virBufferFreeAndReset(&buf);
return -1;
}
+
+ virBufferAsprintf(&buf, ",gic-version=%d", def->gic_version);
}
- }
- }
+ }
+ }
virCommandAddArgBuffer(cmd, &buf);
}
--
Martin
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list