src/qemu/qemu_command.c | 44 +++++++++++++++++------- tests/qemuxml2argvdata/aarch64-gic-none-tcg.args | 2 +- tests/qemuxml2argvdata/aarch64-gic-v2.args | 2 +- tests/qemuxml2argvtest.c | 6 ---- 4 files changed, 33 insertions(+), 21 deletions(-)
Up until now we have only formatted non-default GIC versions on
the command line, in order to maintain compatibility with older
QEMU versions that didn't implement the gic-version option to
begin with; however, doing so is entirely unnecessary for newer
QEMU versions, where the option is available. Moreover, having
the GIC version formatted on the command line at all times
ensures that QEMU changing its own defaults doesn't affect the
ABI of libvirt guests.
A few test cases are removed to avoid extra churn. It doesn't
matter for coverage, as those scenarios are already covered by
other parts of the test suite.
This patch is better viewed with 'git show -w'.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
src/qemu/qemu_command.c | 44 +++++++++++++++++-------
tests/qemuxml2argvdata/aarch64-gic-none-tcg.args | 2 +-
tests/qemuxml2argvdata/aarch64-gic-v2.args | 2 +-
tests/qemuxml2argvtest.c | 6 ----
4 files changed, 33 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 682d714419..1ab1e1a285 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7299,21 +7299,39 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
goto cleanup;
if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON) {
- if (def->gic_version != VIR_GIC_VERSION_NONE) {
- /* The default GIC version (GICv2) should not be specified on
- * the QEMU commandline for backwards compatibility reasons */
- if (def->gic_version != VIR_GIC_VERSION_2) {
- 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"));
- goto cleanup;
- }
+ bool hasGICVersionOption = virQEMUCapsGet(qemuCaps,
+ QEMU_CAPS_MACH_VIRT_GIC_VERSION);
+
+ switch ((virGICVersion) def->gic_version) {
+ case VIR_GIC_VERSION_2:
+ if (!hasGICVersionOption) {
+ /* If the gic-version option is not available, we can't
+ * configure the GIC; however, we know that before the
+ * option was introduced the guests would always get a
+ * GICv2, so in order to maintain compatibility with
+ * those old QEMU versions all we need to do is stop
+ * early instead of erroring out */
+ break;
+ }
+ ATTRIBUTE_FALLTHROUGH;
- virBufferAsprintf(&buf, ",gic-version=%s",
- virGICVersionTypeToString(def->gic_version));
+ case VIR_GIC_VERSION_3:
+ case VIR_GIC_VERSION_HOST:
+ if (!hasGICVersionOption) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("gic-version option is not available "
+ "with this QEMU binary"));
+ goto cleanup;
}
+
+ virBufferAsprintf(&buf, ",gic-version=%s",
+ virGICVersionTypeToString(def->gic_version));
+ break;
+
+ case VIR_GIC_VERSION_NONE:
+ case VIR_GIC_VERSION_LAST:
+ default:
+ break;
}
}
diff --git a/tests/qemuxml2argvdata/aarch64-gic-none-tcg.args b/tests/qemuxml2argvdata/aarch64-gic-none-tcg.args
index 4e3c0eee2d..b766a821d5 100644
--- a/tests/qemuxml2argvdata/aarch64-gic-none-tcg.args
+++ b/tests/qemuxml2argvdata/aarch64-gic-none-tcg.args
@@ -7,7 +7,7 @@ QEMU_AUDIO_DRV=none \
/usr/bin/qemu-system-aarch64 \
-name guest \
-S \
--machine virt,accel=tcg \
+-machine virt,accel=tcg,gic-version=2 \
-cpu cortex-a57 \
-m 1024 \
-smp 1,sockets=1,cores=1,threads=1 \
diff --git a/tests/qemuxml2argvdata/aarch64-gic-v2.args b/tests/qemuxml2argvdata/aarch64-gic-v2.args
index 7e88bbde3f..eec5c4a082 100644
--- a/tests/qemuxml2argvdata/aarch64-gic-v2.args
+++ b/tests/qemuxml2argvdata/aarch64-gic-v2.args
@@ -7,7 +7,7 @@ QEMU_AUDIO_DRV=none \
/usr/bin/qemu-system-aarch64 \
-name aarch64test \
-S \
--machine virt,accel=kvm \
+-machine virt,accel=kvm,gic-version=2 \
-cpu host \
-m 1024 \
-smp 1,sockets=1,cores=1,threads=1 \
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 308d71f725..dee765d3dd 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2608,8 +2608,6 @@ mymain(void)
DO_TEST("aarch64-cpu-passthrough",
QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DEVICE_VIRTIO_MMIO,
QEMU_CAPS_KVM);
- DO_TEST_GIC("aarch64-gic-none", GIC_NONE,
- QEMU_CAPS_KVM, QEMU_CAPS_MACHINE_OPT);
DO_TEST_GIC("aarch64-gic-none", GIC_NONE,
QEMU_CAPS_KVM, QEMU_CAPS_MACHINE_OPT,
QEMU_CAPS_MACH_VIRT_GIC_VERSION);
@@ -2625,8 +2623,6 @@ mymain(void)
DO_TEST_GIC("aarch64-gic-none-tcg", GIC_BOTH,
QEMU_CAPS_MACHINE_OPT,
QEMU_CAPS_MACH_VIRT_GIC_VERSION);
- DO_TEST_GIC("aarch64-gic-default", GIC_NONE,
- QEMU_CAPS_KVM, QEMU_CAPS_MACHINE_OPT);
DO_TEST_GIC("aarch64-gic-default", GIC_NONE,
QEMU_CAPS_KVM, QEMU_CAPS_MACHINE_OPT,
QEMU_CAPS_MACH_VIRT_GIC_VERSION);
@@ -2639,8 +2635,6 @@ mymain(void)
DO_TEST_GIC("aarch64-gic-default-both", GIC_BOTH,
QEMU_CAPS_KVM, QEMU_CAPS_MACHINE_OPT,
QEMU_CAPS_MACH_VIRT_GIC_VERSION);
- DO_TEST_GIC("aarch64-gic-v2", GIC_NONE,
- QEMU_CAPS_KVM, QEMU_CAPS_MACHINE_OPT);
DO_TEST_GIC("aarch64-gic-v2", GIC_NONE,
QEMU_CAPS_KVM, QEMU_CAPS_MACHINE_OPT,
QEMU_CAPS_MACH_VIRT_GIC_VERSION);
--
2.14.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 04/03/2018 08:57 AM, Andrea Bolognani wrote: > Up until now we have only formatted non-default GIC versions on > the command line, in order to maintain compatibility with older > QEMU versions that didn't implement the gic-version option to > begin with; however, doing so is entirely unnecessary for newer > QEMU versions, where the option is available. Moreover, having > the GIC version formatted on the command line at all times > ensures that QEMU changing its own defaults doesn't affect the > ABI of libvirt guests. > > A few test cases are removed to avoid extra churn. It doesn't > matter for coverage, as those scenarios are already covered by > other parts of the test suite. > > This patch is better viewed with 'git show -w'. > > Signed-off-by: Andrea Bolognani <abologna@redhat.com> > --- > src/qemu/qemu_command.c | 44 +++++++++++++++++------- > tests/qemuxml2argvdata/aarch64-gic-none-tcg.args | 2 +- > tests/qemuxml2argvdata/aarch64-gic-v2.args | 2 +- > tests/qemuxml2argvtest.c | 6 ---- > 4 files changed, 33 insertions(+), 21 deletions(-) > And because of code in qemuDomainDefEnableDefaultFeatures related to TCG and GIC and (it seems) bz1414081, the default for TCG would then be v2, hence the reason for the none-tcg change, if I'm reading correctly at least... Although perhaps not obvious just reading this patch ;-) Reviewed-by: John Ferlan <jferlan@redhat.com> John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, 2018-04-11 at 08:52 -0400, John Ferlan wrote: > On 05/03/2018 08:57 AM, Andrea Bolognani wrote: > > Up until now we have only formatted non-default GIC versions on > > the command line, in order to maintain compatibility with older > > QEMU versions that didn't implement the gic-version option to > > begin with; however, doing so is entirely unnecessary for newer > > QEMU versions, where the option is available. Moreover, having > > the GIC version formatted on the command line at all times > > ensures that QEMU changing its own defaults doesn't affect the > > ABI of libvirt guests. > > > > A few test cases are removed to avoid extra churn. It doesn't > > matter for coverage, as those scenarios are already covered by > > other parts of the test suite. > > > > This patch is better viewed with 'git show -w'. > > > > Signed-off-by: Andrea Bolognani <abologna@redhat.com> > > --- > > src/qemu/qemu_command.c | 44 +++++++++++++++++------- > > tests/qemuxml2argvdata/aarch64-gic-none-tcg.args | 2 +- > > tests/qemuxml2argvdata/aarch64-gic-v2.args | 2 +- > > tests/qemuxml2argvtest.c | 6 ---- > > 4 files changed, 33 insertions(+), 21 deletions(-) > > And because of code in qemuDomainDefEnableDefaultFeatures related to TCG > and GIC and (it seems) bz1414081, the default for TCG would then be v2, > hence the reason for the none-tcg change, if I'm reading correctly at > least... Although perhaps not obvious just reading this patch ;-) Yes, that is correct. We're merely making the configuration explicit on the command line, the guest ABI won't change one bit. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.