Re: [libvirt] [PATCH 06/11] conf: Integrate all features ABI checks in the switch

[John Ferlan]

On 02/06/2018 11:42 AM, Andrea Bolognani wrote:
> There are a few stray checks which still live outside of the
> switch in virDomainDefFeaturesCheckABIStability() for no good
> reason. Move them inside the switch, and update the error
> messages to be consistent while at it.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/conf/domain_conf.c | 105 
> -
>  1 file changed, 60 insertions(+), 45 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 9f019c906..170c56665 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -21328,7 +21328,6 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr 
> src,
>  
>  switch ((virDomainFeature) i) {
>  case VIR_DOMAIN_FEATURE_ACPI:
> -case VIR_DOMAIN_FEATURE_APIC:
>  case VIR_DOMAIN_FEATURE_PAE:
>  case VIR_DOMAIN_FEATURE_HAP:
>  case VIR_DOMAIN_FEATURE_VIRIDIAN:
> @@ -21338,10 +21337,7 @@ 
> virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
>  case VIR_DOMAIN_FEATURE_PVSPINLOCK:
>  case VIR_DOMAIN_FEATURE_PMU:
>  case VIR_DOMAIN_FEATURE_VMPORT:
> -case VIR_DOMAIN_FEATURE_GIC:
>  case VIR_DOMAIN_FEATURE_SMM:
> -case VIR_DOMAIN_FEATURE_IOAPIC:
> -case VIR_DOMAIN_FEATURE_HPT:
>  case VIR_DOMAIN_FEATURE_VMCOREINFO:
>  if (src->features[i] != dst->features[i]) {
>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -21366,28 +21362,69 @@ 
> virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
>  }
>  break;
>  
> -case VIR_DOMAIN_FEATURE_LAST:
> +case VIR_DOMAIN_FEATURE_GIC:
> +if (src->features[i] != dst->features[i] ||
> +src->gic_version != dst->gic_version) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("State of feature '%s:%s' differs: "
> + "source: '%s:%s', destination: '%s:%s'"),
> +   featureName, "version",
> +   
> virTristateSwitchTypeToString(src->features[i]),
> +   virGICVersionTypeToString(src->gic_version),
> +   
> virTristateSwitchTypeToString(dst->features[i]),
> +   virGICVersionTypeToString(dst->gic_version));
> +return false;

Similar to previous, should these become "State of feature '%s'
attribute 'version' differs: "

likewise for other error messages Leave it up to you to decide though.

Reviewed-by: John Ferlan 

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 06/11] conf: Integrate all features ABI checks in the switch

[Andrea Bolognani]

There are a few stray checks which still live outside of the
switch in virDomainDefFeaturesCheckABIStability() for no good
reason. Move them inside the switch, and update the error
messages to be consistent while at it.

Signed-off-by: Andrea Bolognani 
---
 src/conf/domain_conf.c | 105 -
 1 file changed, 60 insertions(+), 45 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9f019c906..170c56665 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21328,7 +21328,6 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr 
src,
 
 switch ((virDomainFeature) i) {
 case VIR_DOMAIN_FEATURE_ACPI:
-case VIR_DOMAIN_FEATURE_APIC:
 case VIR_DOMAIN_FEATURE_PAE:
 case VIR_DOMAIN_FEATURE_HAP:
 case VIR_DOMAIN_FEATURE_VIRIDIAN:
@@ -21338,10 +21337,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr 
src,
 case VIR_DOMAIN_FEATURE_PVSPINLOCK:
 case VIR_DOMAIN_FEATURE_PMU:
 case VIR_DOMAIN_FEATURE_VMPORT:
-case VIR_DOMAIN_FEATURE_GIC:
 case VIR_DOMAIN_FEATURE_SMM:
-case VIR_DOMAIN_FEATURE_IOAPIC:
-case VIR_DOMAIN_FEATURE_HPT:
 case VIR_DOMAIN_FEATURE_VMCOREINFO:
 if (src->features[i] != dst->features[i]) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -21366,28 +21362,69 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr 
src,
 }
 break;
 
-case VIR_DOMAIN_FEATURE_LAST:
+case VIR_DOMAIN_FEATURE_GIC:
+if (src->features[i] != dst->features[i] ||
+src->gic_version != dst->gic_version) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("State of feature '%s:%s' differs: "
+ "source: '%s:%s', destination: '%s:%s'"),
+   featureName, "version",
+   virTristateSwitchTypeToString(src->features[i]),
+   virGICVersionTypeToString(src->gic_version),
+   virTristateSwitchTypeToString(dst->features[i]),
+   virGICVersionTypeToString(dst->gic_version));
+return false;
+}
 break;
-}
-}
 
-/* APIC EOI */
-if (src->apic_eoi != dst->apic_eoi) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("State of APIC EOI differs: "
- "source: '%s', destination: '%s'"),
-   virTristateSwitchTypeToString(src->apic_eoi),
-   virTristateSwitchTypeToString(dst->apic_eoi));
-return false;
-}
+case VIR_DOMAIN_FEATURE_HPT:
+if (src->features[i] != dst->features[i] ||
+src->hpt_resizing != dst->hpt_resizing) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("State of feature '%s:%s' differs: "
+ "source: '%s:%s', destination: '%s:%s'"),
+   featureName, "resizing",
+   virTristateSwitchTypeToString(src->features[i]),
+   
virDomainHPTResizingTypeToString(src->hpt_resizing),
+   virTristateSwitchTypeToString(dst->features[i]),
+   
virDomainHPTResizingTypeToString(dst->hpt_resizing));
+return false;
+}
+break;
 
-/* GIC version */
-if (src->gic_version != dst->gic_version) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Source GIC version '%s' does not match destination 
'%s'"),
-   virGICVersionTypeToString(src->gic_version),
-   virGICVersionTypeToString(dst->gic_version));
-return false;
+case VIR_DOMAIN_FEATURE_APIC:
+if (src->features[i] != dst->features[i] ||
+src->apic_eoi != dst->apic_eoi) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("State of feature '%s:%s' differs: "
+ "source: '%s:%s', destination: '%s:%s'"),
+   featureName, "eoi",
+   virTristateSwitchTypeToString(src->features[i]),
+   virTristateSwitchTypeToString(src->apic_eoi),
+   virTristateSwitchTypeToString(dst->features[i]),
+   virTristateSwitchTypeToString(dst->apic_eoi));
+return false;
+}
+break;
+
+case VIR_DOMAIN_FEATURE_IOAPIC:
+if (src->features[i] != dst->features[i] ||
+src->ioapic != dst->ioapic) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+