On 02/03/2016 03:25 PM, Andrea Bolognani wrote:
> Instead of allowing any random positive number, restrict the possible
> values to the ones that are part of the virGICVersion enumeration.
> ---
> src/conf/domain_conf.c | 15 ++++++++-------
> src/conf/domain_conf.h | 3 ++-
> src/libvirt_private.syms | 5 +++++
> src/qemu/qemu_command.c | 8 +++++---
> 4 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 55e7ed9..1785b83 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -15451,8 +15451,8 @@ virDomainDefParseXML(xmlDocPtr xml,
> node = ctxt->node;
> ctxt->node = nodes[i];
> if ((tmp = virXPathString("string(./@version)", ctxt))) {
> - if (virStrToLong_uip(tmp, NULL, 10, &def->gic_version) < 0 ||
> - def->gic_version == 0) {
> + if ((def->gic_version = virGICVersionTypeFromString(tmp)) < 0 ||
> + def->gic_version == VIR_GIC_VERSION_NONE) {
> virReportError(VIR_ERR_XML_ERROR,
> _("malformed gic version: %s"), tmp);
> goto error;
> @@ -17528,8 +17528,9 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
> /* GIC version */
> if (src->gic_version != dst->gic_version) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - _("Source GIC version '%u' does not match destination '%u'"),
> - src->gic_version, dst->gic_version);
> + _("Source GIC version '%s' does not match destination '%s'"),
> + virGICVersionTypeToString(src->gic_version),
> + virGICVersionTypeToString(dst->gic_version));
> return false;
> }
>
> @@ -22206,9 +22207,9 @@ virDomainDefFormatInternal(virDomainDefPtr def,
> case VIR_DOMAIN_FEATURE_GIC:
> if (def->features[i] == VIR_TRISTATE_SWITCH_ON) {
> virBufferAddLit(buf, "<gic");
> - if (def->gic_version)
> - virBufferAsprintf(buf, " version='%u'",
> - def->gic_version);
> + if (def->gic_version != VIR_GIC_VERSION_NONE)
> + virBufferAsprintf(buf, " version='%s'",
> + virGICVersionTypeToString(def->gic_version));
If you went with 'host' being the default from 1/7, then this becomes
optional...
On input the XML would have <gic>; however, on output the XML would be
<gic version='2'> thus tying this domain to using gic v2 unless someone
changes it to '3' (or 'host' or whatever else in the future). Thus your
5/7 patch to change qemuxml2argv-aarch64-aavmf-virtio-mmio.xml
John
> virBufferAddLit(buf, "/>\n");
> }
> break;
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 9fdfdf2..c14857a 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -50,6 +50,7 @@
> # include "virstoragefile.h"
> # include "virseclabel.h"
> # include "virprocess.h"
> +# include "virgic.h"
>
> /* forward declarations of all device types, required by
> * virDomainDeviceDef
> @@ -2262,7 +2263,7 @@ struct _virDomainDef {
> int hyperv_features[VIR_DOMAIN_HYPERV_LAST];
> int kvm_features[VIR_DOMAIN_KVM_LAST];
> unsigned int hyperv_spinlocks;
> - unsigned int gic_version;
> + virGICVersion gic_version;
>
> /* These options are of type virTristateSwitch: ON = keep, OFF = drop */
> int caps_features[VIR_DOMAIN_CAPS_FEATURE_LAST];
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 69be352..8e9c986 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1515,6 +1515,11 @@ virFirewallStartRollback;
> virFirewallStartTransaction;
>
>
> +# util/virgic.h
> +virGICVersionTypeFromString;
> +virGICVersionTypeToString;
> +
> +
> # util/virhash.h
> virHashAddEntry;
> virHashAtomicNew;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 8943270..1f2b142 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -56,6 +56,7 @@
> #include "virtpm.h"
> #include "virscsi.h"
> #include "virnuma.h"
> +#include "virgic.h"
> #if defined(__linux__)
> # include <linux/capability.h>
> #endif
> @@ -8007,7 +8008,7 @@ qemuBuildMachineArgStr(virCommandPtr cmd,
> }
>
> if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON) {
> - if (def->gic_version) {
> + if (def->gic_version != VIR_GIC_VERSION_NONE) {
> if ((def->os.arch != VIR_ARCH_ARMV7L &&
> def->os.arch != VIR_ARCH_AARCH64) ||
> (STRNEQ(def->os.machine, "virt") &&
> @@ -8022,7 +8023,7 @@ qemuBuildMachineArgStr(virCommandPtr cmd,
> /* 2 is the default, so we don't put it as option for
> * backwards compatibility
> */
> - if (def->gic_version != 2) {
> + if (def->gic_version != VIR_GIC_VERSION_2) {
> if (!virQEMUCapsGet(qemuCaps,
> QEMU_CAPS_MACH_VIRT_GIC_VERSION)) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> @@ -8032,7 +8033,8 @@ qemuBuildMachineArgStr(virCommandPtr cmd,
> return -1;
> }
>
> - virBufferAsprintf(&buf, ",gic-version=%d", def->gic_version);
> + virBufferAsprintf(&buf, ",gic-version=%s",
> + virGICVersionTypeToString(def->gic_version));
> }
> }
> }
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list