[libvirt] [PATCH 3/7] conf: Use virGICVersion enumeration in virDomainDef

[John Ferlan]

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));
>                  }
>              }
>          }
>