On Mon, Nov 23, 2015 at 17:41:05 -0500, John Ferlan wrote:
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?
Hmm, yes in this case they should be used. I reordered the patches.
> + 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"?
It should be 0,1,3. Later I'll introduce XML elements that will track
individual vCPUs and allow to transport their state accross to the
destination. Currently this is just preparation code for that and since
there currently is no way to create disjoint vCPU indexes this basically
does the same as the previous code, but in a less optimal fashion.
I can extract just the code as-is and bump the algorithm change to the
patch that will actually be adding this.
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.
Looking forward wouldn't really help. The patches don't exist yet ;)
Conditional ACK depending on response.
Peter