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/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 1676f0e..f9adeb9 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -130,6 +130,31 @@ virDomainCapsEnumSet(virDomainCapsEnumPtr capsEnum,
}
+bool
+virDomainCapsEnumIsSet(const virDomainCapsEnum *capsEnum,
+ const char *capsEnumName,
+ unsigned int value)
+{
+ unsigned int val = 1 << value;
+ bool ret = false;
+
+
Extra newline
+ if (!val) {
+ /* Integer overflow */
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("integer overflow on %s. Please contact the "
+ "libvirt development team at
libvir-list(a)redhat.com"),
+ capsEnumName);
+ goto cleanup;
+ }
+
Heh that's new to me, but I see you're just following the template of
virDomainCapsEnumSet, so okay
+ ret = (capsEnum->values & val);
+
+ cleanup:
+ return ret;
+}
+
+
void
virDomainCapsEnumClear(virDomainCapsEnumPtr capsEnum)
{
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 492a9cf..8ed4111 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -137,10 +137,18 @@ virDomainCapsPtr virDomainCapsNew(const char *path,
__nvalues, __values); \
} while (0)
+# define VIR_DOMAIN_CAPS_ENUM_IS_SET(capsEnum, value) \
+ virDomainCapsEnumIsSet(&(capsEnum), #capsEnum, value) \
+
int virDomainCapsEnumSet(virDomainCapsEnumPtr capsEnum,
const char *capsEnumName,
size_t nvalues,
unsigned int *values);
+
+bool virDomainCapsEnumIsSet(const virDomainCapsEnum *capsEnum,
+ const char *capsEnumName,
+ unsigned int value);
+
void virDomainCapsEnumClear(virDomainCapsEnumPtr capsEnum);
char * virDomainCapsFormat(virDomainCapsPtr const caps);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fff8c30..183e84a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -141,6 +141,7 @@ virDomainAuditVcpu;
# conf/domain_capabilities.h
virDomainCapsEnumClear;
+virDomainCapsEnumIsSet;
virDomainCapsEnumSet;
virDomainCapsFormat;
virDomainCapsNew;
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?
- Cole
- 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;
}
/* 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;
+
+ ret = 0;
+
+ cleanup:
+ virObjectUnref(caps);
+
+ return ret;
}
@@ -2033,7 +2074,8 @@ qemuDomainDefPostParse(virDomainDefPtr def,
if (qemuCanonicalizeMachine(def, qemuCaps) < 0)
goto cleanup;
- qemuDomainDefEnableDefaultFeatures(def);
+ if (qemuDomainDefEnableDefaultFeatures(def, qemuCaps) < 0)
+ goto cleanup;
qemuDomainRecheckInternalPaths(def, cfg, parseFlags);