On 11/20/2015 10:22 AM, Peter Krempa wrote:
Extract the checking code into a separate function and prepare the infrastructure for checking the new structure type. --- src/conf/domain_conf.c | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 631e1db..66fc6d3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17835,6 +17835,35 @@ virDomainMemoryDefCheckABIStability(virDomainMemoryDefPtr src, }
+static bool +virDomainDefVcpuCheckAbiStability(virDomainDefPtr src, + virDomainDefPtr dst)
I see use of "Vcpu" here...
+{ + size_t i; + + if (src->maxvcpus != dst->maxvcpus) {
Should these be accessors? Like they were in the moved code?
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target domain vCPU max %zu does not match source %zu"), + dst->maxvcpus, src->maxvcpus); + return false; + } + + for (i = 0; i < src->maxvcpus; i++) {
Allowing for this to be an accessor/local too.
+ virDomainVCpuInfoPtr svcpu = &src->vcpus[i]; + virDomainVCpuInfoPtr dvcpu = &dst->vcpus[i]; + + if (svcpu->online != dvcpu->online) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("State of vCPU '%zu' differs between source and " + "destination definitions"), i); + return false; + }
This changes the original code/design just slightly from a counted value online to having the same order between source/dest. If the current algorithm of using the first 'current' vcpus doesn't change, then I foresee perhaps an interesting issue/question. Say we start with 2 current (0,1) and 4 total (0,1,2,3). If we allow someone to start/hotplug #3, then a migration occurs. Would the "target" start "0,1,2" or "0,1,3"? If I think about the current algorithm, it's get the # of vCPU's "current" (virDomainDefGetVCpus) and then set online in order 0, 1, 2 (virDomainDefSetVCpus). That causes a failure for this algorithm, but should it? Again only an issue if you're ultimate goal is to allow the user to choose which vCPU's to place online or offline. I haven't looked that far forward yet. Conditional ACK depending on response. John
+ } + + return true; +} + + /* This compares two configurations and looks for any differences * which will affect the guest ABI. This is primarily to allow * validation of custom XML config passed in during migration @@ -17908,18 +17937,8 @@ virDomainDefCheckABIStability(virDomainDefPtr src, goto error; }
- if (virDomainDefGetVCpus(src) != virDomainDefGetVCpus(dst)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain vCPU count %d does not match source %d"), - virDomainDefGetVCpus(dst), virDomainDefGetVCpus(src)); + if (!virDomainDefVcpuCheckAbiStability(src, dst)) goto error; - } - if (virDomainDefGetVCpusMax(src) != virDomainDefGetVCpusMax(dst)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain vCPU max %d does not match source %d"), - virDomainDefGetVCpusMax(dst), virDomainDefGetVCpusMax(src)); - goto error; - }
if (src->iothreads != dst->iothreads) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED,