On 11.05.2016 00:42, Cole Robinson wrote:
> On 05/10/2016 08:46 AM, 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/conf/domain_capabilities.c | 25 ++++++++++++++++
>> src/conf/domain_capabilities.h | 8 +++++
>> src/libvirt_private.syms | 1 +
>> src/qemu/qemu_domain.c | 66 ++++++++++++++++++++++++++++++++++--------
>> 4 files changed, 88 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 93f0a01..d34450f 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -1921,26 +1921,67 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
>> * Make sure that features that should be enabled by default are actually
>> * enabled and configure default values related to those features.
>> */
>> -static void
>> -qemuDomainDefEnableDefaultFeatures(virDomainDefPtr def)
>> +static int
>> +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;
>> + virDomainCapsPtr caps = NULL;
>> + virDomainCapsFeatureGICPtr gic = NULL;
>> + int ret = -1;
>> +
>> + /* 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 */
>> + if ((def->os.arch == VIR_ARCH_ARMV7L ||
>> + def->os.arch == VIR_ARCH_AARCH64) &&
>> + qemuDomainMachineIsVirt(def) &&
>> + def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ABSENT) {
>> +
>> + if (!(caps = virDomainCapsNew(def->emulator,
>> + def->os.machine,
>> + def->os.arch,
>> + def->virtType)))
>> + goto cleanup;
>> +
>
> Call this domCaps ? 'caps' name is usually used for virCapabilities
>
>> + if (virQEMUCapsFillDomainCaps(caps, qemuCaps, NULL, 0) < 0)
>> + goto cleanup;
>> +
>> + gic = &(caps->gic);
>> +
>> + /* Pick the best GIC version from those available */
>> + if (gic->supported) {
>> + virGICVersion version;
>> +
>> + VIR_DEBUG("Looking for usable GIC version in domain capabilities");
>> + for (version = VIR_GIC_VERSION_LAST - 1;
>> + version > VIR_GIC_VERSION_NONE;
>> + version--) {
>> + if (VIR_DOMAIN_CAPS_ENUM_IS_SET(gic->version, version)) {
>> +
>> + VIR_DEBUG("Using GIC version %s",
>> + virGICVersionTypeToString(version));
>> + def->gic_version = version;
>> + break;
>> + }
>> + }
>> }
>
> Hmm that's a bit of a new pattern... it seems the only thing you really need
> from domcaps is the bit of logic we encode via
> virQEMUCapsFillDomainFeatureGICCaps. Maybe break that logic out into a public
> function and call it here, rather than spinning up domcaps for a small bit of
> info? Or is there more to it?
Agreed. This looks like too heavy hammer for nail this small.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list