From: Michal Privoznik <mprivozn@redhat.com> So far, we have an array of integers (hyperv_features), an uint (hyperv_spinlocks), a string (hyperv_vendor_id) and some tristate switches scattered across virDomainDef. Soon, new knobs will be introduced and keeping the current state would only worsen readability. Introduce virDomainHypervFeatures struct to place hyperv related features there. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 77 ++++++++++++++++++++++------------------ src/conf/domain_conf.h | 15 ++++---- src/conf/virconftypes.h | 2 ++ src/libxl/libxl_conf.c | 2 +- src/qemu/qemu_command.c | 16 ++++----- src/qemu/qemu_process.c | 8 ++--- src/qemu/qemu_validate.c | 12 +++---- 7 files changed, 72 insertions(+), 60 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d095454283..8c42f95af4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4020,6 +4020,13 @@ virDomainOSDefClear(virDomainOSDef *os) } +static void +virDomainHypervFeaturesClear(virDomainHypervFeatures *hv) +{ + g_free(hv->vendor_id); +} + + void virDomainDefFree(virDomainDef *def) { size_t i; @@ -4147,8 +4154,8 @@ void virDomainDefFree(virDomainDef *def) g_free(def->emulator); g_free(def->description); g_free(def->title); + virDomainHypervFeaturesClear(&def->hyperv); g_free(def->kvm_features); - g_free(def->hyperv_vendor_id); g_free(def->tcg_features); virBlkioDeviceArrayClear(def->blkio.devices, @@ -17049,7 +17056,7 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, &value) < 0) return -1; - def->hyperv_features[feature] = value; + def->hyperv.features[feature] = value; switch ((virDomainHyperv) feature) { case VIR_DOMAIN_HYPERV_RELAXED: @@ -17075,11 +17082,11 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, while (child) { if (STREQ((const char *)child->name, "direct")) { if (virXMLPropTristateSwitch(child, "state", VIR_XML_PROP_REQUIRED, - &def->hyperv_tlbflush_direct) < 0) + &def->hyperv.tlbflush_direct) < 0) return -1; } else if (STREQ((const char *)child->name, "extended")) { if (virXMLPropTristateSwitch(child, "state", VIR_XML_PROP_REQUIRED, - &def->hyperv_tlbflush_extended) < 0) + &def->hyperv.tlbflush_extended) < 0) return -1; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -17106,7 +17113,7 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, } if (virXMLPropTristateSwitch(child, "state", VIR_XML_PROP_REQUIRED, - &def->hyperv_stimer_direct) < 0) + &def->hyperv.stimer_direct) < 0) return -1; child = xmlNextElementSibling(child); @@ -17118,10 +17125,10 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, break; if (virXMLPropUInt(node, "retries", 0, VIR_XML_PROP_REQUIRED, - &def->hyperv_spinlocks) < 0) + &def->hyperv.spinlocks) < 0) return -1; - if (def->hyperv_spinlocks < 0xFFF) { + if (def->hyperv.spinlocks < 0xFFF) { virReportError(VIR_ERR_XML_ERROR, "%s", _("HyperV spinlock retry count must be at least 4095")); return -1; @@ -17132,15 +17139,15 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, if (value != VIR_TRISTATE_SWITCH_ON) break; - g_clear_pointer(&def->hyperv_vendor_id, g_free); + g_clear_pointer(&def->hyperv.vendor_id, g_free); - if (!(def->hyperv_vendor_id = virXMLPropString(node, "value"))) { + if (!(def->hyperv.vendor_id = virXMLPropString(node, "value"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing 'value' attribute for HyperV feature 'vendor_id'")); return -1; } - if (!STRLIM(def->hyperv_vendor_id, VIR_DOMAIN_HYPERV_VENDOR_ID_MAX)) { + if (!STRLIM(def->hyperv.vendor_id, VIR_DOMAIN_HYPERV_VENDOR_ID_MAX)) { virReportError(VIR_ERR_XML_ERROR, _("HyperV vendor_id value must not be more than %1$d characters."), VIR_DOMAIN_HYPERV_VENDOR_ID_MAX); @@ -17148,7 +17155,7 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, } /* ensure that the string can be passed to qemu */ - if (strchr(def->hyperv_vendor_id, ',')) { + if (strchr(def->hyperv.vendor_id, ',')) { virReportError(VIR_ERR_XML_ERROR, "%s", _("HyperV vendor_id value is invalid")); return -1; @@ -21641,12 +21648,12 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, case VIR_DOMAIN_HYPERV_AVIC: case VIR_DOMAIN_HYPERV_EMSR_BITMAP: case VIR_DOMAIN_HYPERV_XMM_INPUT: - if (src->hyperv_features[i] != dst->hyperv_features[i]) { + if (src->hyperv.features[i] != dst->hyperv.features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of HyperV enlightenment feature '%1$s' differs: source: '%2$s', destination: '%3$s'"), virDomainHypervTypeToString(i), - virTristateSwitchTypeToString(src->hyperv_features[i]), - virTristateSwitchTypeToString(dst->hyperv_features[i])); + virTristateSwitchTypeToString(src->hyperv.features[i]), + virTristateSwitchTypeToString(dst->hyperv.features[i])); return false; } @@ -21654,21 +21661,21 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, case VIR_DOMAIN_HYPERV_SPINLOCKS: /* spinlock count matters! */ - if (src->hyperv_spinlocks != dst->hyperv_spinlocks) { + if (src->hyperv.spinlocks != dst->hyperv.spinlocks) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("HyperV spinlock retry count differs: source: '%1$u', destination: '%2$u'"), - src->hyperv_spinlocks, - dst->hyperv_spinlocks); + src->hyperv.spinlocks, + dst->hyperv.spinlocks); return false; } break; case VIR_DOMAIN_HYPERV_VENDOR_ID: - if (STRNEQ_NULLABLE(src->hyperv_vendor_id, dst->hyperv_vendor_id)) { + if (STRNEQ_NULLABLE(src->hyperv.vendor_id, dst->hyperv.vendor_id)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("HyperV vendor_id differs: source: '%1$s', destination: '%2$s'"), - src->hyperv_vendor_id, - dst->hyperv_vendor_id); + src->hyperv.vendor_id, + dst->hyperv.vendor_id); return false; } break; @@ -21679,12 +21686,12 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, } } - if (src->hyperv_features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { - if (src->hyperv_stimer_direct != dst->hyperv_stimer_direct) { + if (src->hyperv.features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { + if (src->hyperv.stimer_direct != dst->hyperv.stimer_direct) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of HyperV stimer direct feature differs: source: '%1$s', destination: '%2$s'"), - virTristateSwitchTypeToString(src->hyperv_stimer_direct), - virTristateSwitchTypeToString(dst->hyperv_stimer_direct)); + virTristateSwitchTypeToString(src->hyperv.stimer_direct), + virTristateSwitchTypeToString(dst->hyperv.stimer_direct)); return false; } } @@ -28599,11 +28606,11 @@ virDomainFeaturesHyperVDefFormat(virBuffer *buf, g_auto(virBuffer) hypervAttrBuf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) hypervChildBuf = VIR_BUFFER_INIT_CHILD(&childBuf); - if (def->hyperv_features[j] == VIR_TRISTATE_SWITCH_ABSENT) + if (def->hyperv.features[j] == VIR_TRISTATE_SWITCH_ABSENT) continue; virBufferAsprintf(&hypervAttrBuf, " state='%s'", - virTristateSwitchTypeToString(def->hyperv_features[j])); + virTristateSwitchTypeToString(def->hyperv.features[j])); switch ((virDomainHyperv) j) { case VIR_DOMAIN_HYPERV_RELAXED: @@ -28622,34 +28629,34 @@ virDomainFeaturesHyperVDefFormat(virBuffer *buf, break; case VIR_DOMAIN_HYPERV_SPINLOCKS: - if (def->hyperv_features[j] == VIR_TRISTATE_SWITCH_ON) { + if (def->hyperv.features[j] == VIR_TRISTATE_SWITCH_ON) { virBufferAsprintf(&hypervAttrBuf, - " retries='%d'", def->hyperv_spinlocks); + " retries='%d'", def->hyperv.spinlocks); } break; case VIR_DOMAIN_HYPERV_STIMER: - if (def->hyperv_features[j] == VIR_TRISTATE_SWITCH_ON && - def->hyperv_stimer_direct == VIR_TRISTATE_SWITCH_ON) { + if (def->hyperv.features[j] == VIR_TRISTATE_SWITCH_ON && + def->hyperv.stimer_direct == VIR_TRISTATE_SWITCH_ON) { virBufferAddLit(&hypervChildBuf, "<direct state='on'/>\n"); } break; case VIR_DOMAIN_HYPERV_VENDOR_ID: - if (def->hyperv_features[j] == VIR_TRISTATE_SWITCH_ON) { + if (def->hyperv.features[j] == VIR_TRISTATE_SWITCH_ON) { virBufferEscapeString(&hypervAttrBuf, " value='%s'", - def->hyperv_vendor_id); + def->hyperv.vendor_id); } break; case VIR_DOMAIN_HYPERV_TLBFLUSH: - if (def->hyperv_features[j] != VIR_TRISTATE_SWITCH_ON) + if (def->hyperv.features[j] != VIR_TRISTATE_SWITCH_ON) break; - if (def->hyperv_tlbflush_direct == VIR_TRISTATE_SWITCH_ON) + if (def->hyperv.tlbflush_direct == VIR_TRISTATE_SWITCH_ON) virBufferAddLit(&hypervChildBuf, "<direct state='on'/>\n"); - if (def->hyperv_tlbflush_extended == VIR_TRISTATE_SWITCH_ON) + if (def->hyperv.tlbflush_extended == VIR_TRISTATE_SWITCH_ON) virBufferAddLit(&hypervChildBuf, "<extended state='on'/>\n"); break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9159a69833..5e37ef7b0d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3124,6 +3124,14 @@ struct _virDomainPstoreDef { virDomainDeviceInfo info; }; +struct _virDomainHypervFeatures { + int features[VIR_DOMAIN_HYPERV_LAST]; + unsigned int spinlocks; + virTristateSwitch stimer_direct; + virTristateSwitch tlbflush_direct; + virTristateSwitch tlbflush_extended; + char *vendor_id; +}; #define SCSI_SUPER_WIDE_BUS_MAX_CONT_UNIT 64 #define SCSI_WIDE_BUS_MAX_CONT_UNIT 16 @@ -3195,19 +3203,14 @@ struct _virDomainDef { * See virDomainDefFeaturesCheckABIStability() for details. */ int features[VIR_DOMAIN_FEATURE_LAST]; int caps_features[VIR_DOMAIN_PROCES_CAPS_FEATURE_LAST]; - int hyperv_features[VIR_DOMAIN_HYPERV_LAST]; + virDomainHypervFeatures hyperv; virDomainFeatureKVM *kvm_features; int msrs_features[VIR_DOMAIN_MSRS_LAST]; int xen_features[VIR_DOMAIN_XEN_LAST]; virDomainXenPassthroughMode xen_passthrough_mode; - unsigned int hyperv_spinlocks; - virTristateSwitch hyperv_stimer_direct; - virTristateSwitch hyperv_tlbflush_direct; - virTristateSwitch hyperv_tlbflush_extended; virGICVersion gic_version; virDomainHPTResizing hpt_resizing; unsigned long long hpt_maxpagesize; /* Stored in KiB */ - char *hyperv_vendor_id; virTristateSwitch apic_eoi; virDomainFeatureTCG *tcg_features; diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index 93fc9c9217..6e2573035a 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -142,6 +142,8 @@ typedef struct _virDomainHubDef virDomainHubDef; typedef struct _virDomainHugePage virDomainHugePage; +typedef struct _virDomainHypervFeatures virDomainHypervFeatures; + typedef struct _virDomainIOMMUDef virDomainIOMMUDef; typedef struct _virDomainIOThreadIDDef virDomainIOThreadIDDef; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index cea4bd801c..3962a7dba2 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -608,7 +608,7 @@ libxlMakeDomBuildInfo(virDomainDef *def, } for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) { - if (def->hyperv_features[i] != VIR_TRISTATE_SWITCH_ON) + if (def->hyperv.features[i] != VIR_TRISTATE_SWITCH_ON) continue; switch ((virDomainHyperv) i) { diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 65b50740ce..7a31848d6f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6492,7 +6492,7 @@ qemuBuildCpuHypervCommandLine(virBuffer *buf, case VIR_DOMAIN_HYPERV_AVIC: case VIR_DOMAIN_HYPERV_EMSR_BITMAP: case VIR_DOMAIN_HYPERV_XMM_INPUT: - if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) { + if (def->hyperv.features[i] == VIR_TRISTATE_SWITCH_ON) { const char *name = virDomainHypervTypeToString(i); g_autofree char *full_name = g_strdup_printf("hv-%s", name); const char *qemu_name = virQEMUCapsCPUFeatureToQEMU(def->os.arch, @@ -6500,27 +6500,27 @@ qemuBuildCpuHypervCommandLine(virBuffer *buf, virBufferAsprintf(buf, ",%s=on", qemu_name); } if ((i == VIR_DOMAIN_HYPERV_STIMER) && - (def->hyperv_stimer_direct == VIR_TRISTATE_SWITCH_ON)) + (def->hyperv.stimer_direct == VIR_TRISTATE_SWITCH_ON)) virBufferAsprintf(buf, ",%s=on", VIR_CPU_x86_HV_STIMER_DIRECT); if (i == VIR_DOMAIN_HYPERV_TLBFLUSH) { - if (def->hyperv_tlbflush_direct == VIR_TRISTATE_SWITCH_ON) + if (def->hyperv.tlbflush_direct == VIR_TRISTATE_SWITCH_ON) virBufferAsprintf(buf, ",%s=on", VIR_CPU_x86_HV_TLBFLUSH_DIRECT); - if (def->hyperv_tlbflush_extended == VIR_TRISTATE_SWITCH_ON) + if (def->hyperv.tlbflush_extended == VIR_TRISTATE_SWITCH_ON) virBufferAsprintf(buf, ",%s=on", VIR_CPU_x86_HV_TLBFLUSH_EXT); } break; case VIR_DOMAIN_HYPERV_SPINLOCKS: - if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) + if (def->hyperv.features[i] == VIR_TRISTATE_SWITCH_ON) virBufferAsprintf(buf, ",%s=0x%x", VIR_CPU_x86_HV_SPINLOCKS, - def->hyperv_spinlocks); + def->hyperv.spinlocks); break; case VIR_DOMAIN_HYPERV_VENDOR_ID: - if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) + if (def->hyperv.features[i] == VIR_TRISTATE_SWITCH_ON) virBufferAsprintf(buf, ",hv-vendor-id=%s", - def->hyperv_vendor_id); + def->hyperv.vendor_id); break; case VIR_DOMAIN_HYPERV_LAST: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 57b14f640e..d42e3f612b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4418,7 +4418,7 @@ qemuProcessVerifyHypervFeatures(virDomainDef *def, i == VIR_DOMAIN_HYPERV_SPINLOCKS) continue; - if (def->hyperv_features[i] != VIR_TRISTATE_SWITCH_ON) + if (def->hyperv.features[i] != VIR_TRISTATE_SWITCH_ON) continue; cpuFeature = g_strdup_printf("hv-%s", virDomainHypervTypeToString(i)); @@ -4429,7 +4429,7 @@ qemuProcessVerifyHypervFeatures(virDomainDef *def, return -1; } else if (rc == 1) { if (i == VIR_DOMAIN_HYPERV_STIMER) { - if (def->hyperv_stimer_direct != VIR_TRISTATE_SWITCH_ON) + if (def->hyperv.stimer_direct != VIR_TRISTATE_SWITCH_ON) continue; rc = virCPUDataCheckFeature(cpu, VIR_CPU_x86_HV_STIMER_DIRECT); @@ -4444,7 +4444,7 @@ qemuProcessVerifyHypervFeatures(virDomainDef *def, return -1; } if (i == VIR_DOMAIN_HYPERV_TLBFLUSH) { - if (def->hyperv_tlbflush_direct == VIR_TRISTATE_SWITCH_ON) { + if (def->hyperv.tlbflush_direct == VIR_TRISTATE_SWITCH_ON) { rc = virCPUDataCheckFeature(cpu, VIR_CPU_x86_HV_TLBFLUSH_DIRECT); if (rc < 0) return -1; @@ -4455,7 +4455,7 @@ qemuProcessVerifyHypervFeatures(virDomainDef *def, return -1; } } - if (def->hyperv_tlbflush_extended == VIR_TRISTATE_SWITCH_ON) { + if (def->hyperv.tlbflush_extended == VIR_TRISTATE_SWITCH_ON) { rc = virCPUDataCheckFeature(cpu, VIR_CPU_x86_HV_TLBFLUSH_EXT); if (rc < 0) return -1; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 930ba3543f..3e8fdb2268 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -89,8 +89,8 @@ qemuValidateDomainDefPSeriesFeature(const virDomainDef *def, #define CHECK_HV_FEAT(feat, requires) \ - if (def->hyperv_features[feat] == VIR_TRISTATE_SWITCH_ON && \ - def->hyperv_features[requires] != VIR_TRISTATE_SWITCH_ON) { \ + if (def->hyperv.features[feat] == VIR_TRISTATE_SWITCH_ON && \ + def->hyperv.features[requires] != VIR_TRISTATE_SWITCH_ON) { \ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \ _("'%1$s' hyperv feature requires '%2$s' feature"), \ virDomainHypervTypeToString(feat), \ @@ -114,7 +114,7 @@ qemuValidateDomainDefHypervFeatures(const virDomainDef *def) CHECK_HV_FEAT(VIR_DOMAIN_HYPERV_SYNIC, VIR_DOMAIN_HYPERV_VPINDEX); - if (def->hyperv_features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { + if (def->hyperv.features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { if (!virDomainDefHasTimer(def, VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("'%1$s' hyperv feature requires '%2$s' timer"), @@ -133,9 +133,9 @@ qemuValidateDomainDefHypervFeatures(const virDomainDef *def) CHECK_HV_FEAT(VIR_DOMAIN_HYPERV_EVMCS, VIR_DOMAIN_HYPERV_VAPIC); - if (def->hyperv_features[VIR_DOMAIN_HYPERV_TLBFLUSH] == VIR_TRISTATE_SWITCH_ON && - def->hyperv_tlbflush_direct == VIR_TRISTATE_SWITCH_ON) { - if (def->hyperv_features[VIR_DOMAIN_HYPERV_VAPIC] != VIR_TRISTATE_SWITCH_ON) { + if (def->hyperv.features[VIR_DOMAIN_HYPERV_TLBFLUSH] == VIR_TRISTATE_SWITCH_ON && + def->hyperv.tlbflush_direct == VIR_TRISTATE_SWITCH_ON) { + if (def->hyperv.features[VIR_DOMAIN_HYPERV_VAPIC] != VIR_TRISTATE_SWITCH_ON) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("'%1$s' hyperv feature requires '%2$s' feature"), VIR_CPU_x86_HV_TLBFLUSH_DIRECT, -- 2.49.1