[libvirt] [PATCH 10/11] conf: Improve HPT feature handling

[John Ferlan]

On 02/06/2018 11:42 AM, Andrea Bolognani wrote:
> Instead of storing separately whether the feature is enabled
> or not and what resizing policy should be used, store both of
> them in a single place.
> 
> Signed-off-by: Andrea Bolognani <abologna at redhat.com>
> ---
>  src/conf/domain_conf.c  | 26 ++++++++++++--------------
>  src/conf/domain_conf.h  |  4 ++--
>  src/qemu/qemu_command.c |  4 ++--
>  src/qemu/qemu_domain.c  |  2 +-
>  4 files changed, 17 insertions(+), 19 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 19884ec13..c1d549594 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -906,6 +906,7 @@ VIR_ENUM_IMPL(virDomainIOAPIC,
>  
>  VIR_ENUM_IMPL(virDomainHPTResizing,
>                VIR_DOMAIN_HPT_RESIZING_LAST,
> +              "none",
>                "enabled",
>                "disabled",
>                "required",
> @@ -19239,14 +19240,13 @@ virDomainDefParseXML(xmlDocPtr xml,
>              tmp = virXMLPropString(nodes[i], "resizing");
>              if (tmp) {
>                  int value = virDomainHPTResizingTypeFromString(tmp);
> -                if (value < 0) {
> +                if (value < 0 || value == VIR_DOMAIN_HPT_RESIZING_NONE) {

Similar note as w/ previous and rng schema. Here too I'm fine with this
way, but if someone else isn't hopefully they speak up.

>                      virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                                     _("Unknown HPT resizing setting: %s"),
>                                     tmp);
>                      goto error;
>                  }
> -                def->hpt_resizing = value;
> -                def->features[val] = VIR_TRISTATE_SWITCH_ON;
> +                def->features[val] = value;
>                  VIR_FREE(tmp);
>              }
>              break;
> @@ -21377,16 +21377,13 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
>              break;
>  
>          case VIR_DOMAIN_FEATURE_HPT:
> -            if (src->features[i] != dst->features[i] ||
> -                src->hpt_resizing != dst->hpt_resizing) {
> +            if (src->features[i] != dst->features[i]) {
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                                 _("State of feature '%s:%s' differs: "
> -                                 "source: '%s:%s', destination: '%s:%s'"),
> +                                 "source: '%s', destination: '%s'"),
>                                 featureName, "resizing",
> -                               virTristateSwitchTypeToString(src->features[i]),
> -                               virDomainHPTResizingTypeToString(src->hpt_resizing),
> -                               virTristateSwitchTypeToString(dst->features[i]),
> -                               virDomainHPTResizingTypeToString(dst->hpt_resizing));
> +                               virDomainHPTResizingTypeToString(src->features[i]),
> +                               virDomainHPTResizingTypeToString(dst->features[i]));
>                  return false;
>              }
>              break;
> @@ -26947,10 +26944,11 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>                  break;
>  
>              case VIR_DOMAIN_FEATURE_HPT:
> -                if (def->features[i] == VIR_TRISTATE_SWITCH_ON) {
> -                    virBufferAsprintf(buf, "<hpt resizing='%s'/>\n",
> -                                      virDomainHPTResizingTypeToString(def->hpt_resizing));
> -                }
> +                if (def->features[i] == VIR_DOMAIN_HPT_RESIZING_NONE)
> +                    break;
> +
> +                virBufferAsprintf(buf, "<hpt resizing='%s'/>\n",
> +                                  virDomainHPTResizingTypeToString(def->features[i]));
>                  break;
>  
>              /* coverity[dead_error_begin] */
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 20f0efc36..4e9044ae6 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1871,7 +1871,8 @@ typedef enum {
>  VIR_ENUM_DECL(virDomainIOAPIC);
>  
>  typedef enum {
> -    VIR_DOMAIN_HPT_RESIZING_ENABLED = 0,
> +    VIR_DOMAIN_HPT_RESIZING_NONE = 0,
> +    VIR_DOMAIN_HPT_RESIZING_ENABLED,
>      VIR_DOMAIN_HPT_RESIZING_DISABLED,
>      VIR_DOMAIN_HPT_RESIZING_REQUIRED,
>  
> @@ -2364,7 +2365,6 @@ struct _virDomainDef {

The comment a few lines above here should be updated to include:

 * VIR_DOMAIN_FEATURE_HPT is of type virDomainHPTResizing

Reviewed-by: John Ferlan <jferlan at redhat.com>

John

>      unsigned int hyperv_spinlocks;
>      virGICVersion gic_version;
>      char *hyperv_vendor_id;
> -    virDomainHPTResizing hpt_resizing;
>  
>      /* These options are of type virTristateSwitch: ON = keep, OFF = drop */
>      int caps_features[VIR_DOMAIN_CAPS_FEATURE_LAST];

[...]