On 05/16/2016 06:00 PM, Andrea Bolognani wrote:
> When the <gic/> element in not present in the domain XML, use the
> domain capabilities to figure out what GIC version is usable and
> choose that one automatically.
>
> This allows guests to be created on hardware that only supports
> GIC v3 without having to update virt-manager and similar tools.
>
> Keep using the default GIC version if the <gic/> element has been
> added to the domain XML but no version has been specified, as not
> to break existing guests.
> ---
> src/qemu/qemu_domain.c | 49 +++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 37 insertions(+), 12 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index b0eb3b6..ec59763 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1945,25 +1945,50 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
Document @qemuCaps
> * enabled and configure default values related to those features.
> */
> static void
> -qemuDomainDefEnableDefaultFeatures(virDomainDefPtr def)
> +qemuDomainDefEnableDefaultFeatures(virDomainDefPtr def,
> + virQEMUCapsPtr qemuCaps)
> {
> - switch (def->os.arch) {
> - case VIR_ARCH_ARMV7L:
> - case VIR_ARCH_AARCH64:
> - if (qemuDomainMachineIsVirt(def)) {
> - /* GIC is always available to ARM virt machines */
> - def->features[VIR_DOMAIN_FEATURE_GIC] = VIR_TRISTATE_SWITCH_ON;
> + do {
Not a fan... Isn't what you're doing the same as:
if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ABSENT &&
(def->os.arch == VIR_ARCH_ARMV7L || def->os.arch == VIR_ARCH_AARCH64) &&
qemuDomainMachineIsVirt(def)) {
> + virGICVersion version;
> +
> + if (def->features[VIR_DOMAIN_FEATURE_GIC] != VIR_TRISTATE_SWITCH_ABSENT)
> + break;
> +
> + if (def->os.arch != VIR_ARCH_ARMV7L &&
> + def->os.arch != VIR_ARCH_AARCH64)
> + break;
> +
> + if (!qemuDomainMachineIsVirt(def))
> + break;
removing these checks
> +
> + /* The virt machine type always uses GIC: if the relevant element
> + * was not included in the domain XML, we need to choose a suitable
> + * GIC version ourselves */
> + VIR_DEBUG("Looking for usable GIC version in domain capabilities");
> + for (version = VIR_GIC_VERSION_LAST - 1;
> + version > VIR_GIC_VERSION_NONE;
> + version--) {
> + if (virQEMUCapsSupportsGICVersion(qemuCaps,
> + def->virtType,
> + version)) {
> + VIR_DEBUG("Using GIC version %s",
> + virGICVersionTypeToString(version));
> + def->gic_version = version;
> + break;
> + }
> }
> - break;
>
> - default:
> - break;
> - }
> + /* Even if we haven't found a usable GIC version in the domain
> + * capabilities, we still want to enable this */
> + def->features[VIR_DOMAIN_FEATURE_GIC] = VIR_TRISTATE_SWITCH_ON;
> + } while (false);
Removing the " while (false);"
>
> /* Use the default GIC version if no version was specified */
> if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON &&
> def->gic_version == VIR_GIC_VERSION_NONE)
> def->gic_version = VIR_GIC_VERSION_DEFAULT;
> +
> + return;
Unnecessary for void function can be removed anyway
ACK with the adjustments
John
> }
>
>
> @@ -2056,7 +2081,7 @@ qemuDomainDefPostParse(virDomainDefPtr def,
> if (qemuCanonicalizeMachine(def, qemuCaps) < 0)
> goto cleanup;
>
> - qemuDomainDefEnableDefaultFeatures(def);
> + qemuDomainDefEnableDefaultFeatures(def, qemuCaps);
>
> qemuDomainRecheckInternalPaths(def, cfg, parseFlags);
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list