Re: [libvirt] [PATCH 27/34] conf: ABI: Split up and improve vcpu info ABI checking
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,