[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] cpu: break out when a right cpuCandidate found




> -----Original Message-----
> From: Martin Kletzander [mailto:mkletzan redhat com]
> Sent: Thursday, February 13, 2014 6:08 PM
> To: Wangyufei (James)
> Cc: libvir-list redhat com; Wangrui (K); Zhaoyanbin (A)
> Subject: Re: [libvirt] [PATCH] cpu: break out when a right cpuCandidate found
> 
> On Thu, Feb 13, 2014 at 07:44:20AM +0000, Wangyufei (James) wrote:
> > >From 8123c5d64f940fa0fb0de32fc5e68035980b6b01 Mon Sep 17
> 00:00:00 2001
> > From: WangYufei <james wangyufei huawei com>
> > Date: Thu, 13 Feb 2014 07:17:11 +0000
> > Subject: [PATCH] cpu: break out when a right cpuCandidate found
> >
> > In function x86Decode there's a code segment in while cycle like this:
> >         if (cpuModel == NULL
> >             || cpuModel->nfeatures > cpuCandidate->nfeatures) {
> >             virCPUDefFree(cpuModel);
> >             cpuModel = cpuCandidate;
> >             cpuData = candidate->data;
> >         } else {
> >             virCPUDefFree(cpuCandidate);
> >         }
> > when it finds the right cpuCandidate, it doesn't break out the cycle, but
> continues
> > run in it, and cpuModel will never get a new value, it's meaningless. It
> should
> > break out when a right cpuCndidate found.
> >
> 
> Inside this condition, the code doesn't always choose the perfect
> candidate.  You don't consider a situation when the cycle continues
> and the next candidate model is the preferred one, thus satisfies
> previous condition, which looks like this:
> 
> if (preferred && STREQ(cpuCandidate->model, preferred)) {
>     virCPUDefFree(cpuModel);
>     cpuModel = cpuCandidate;
>     cpuData = candidate->data;
>     break;
> }
> 
> Where the "perfect" cpuModel is found, used and the condition breaks
> (appropriately this time).  But I could also misunderstood the code.
> 
Well, thank you for your reply. I have seen preferred, but there's no where to modify the value of preferred in the cycle.
So the preferred is a fixed value. In this case, I can make it better to mdify the patch like this:
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -1558,6 +1558,7 @@ x86Decode(virCPUDefPtr cpu,
             virCPUDefFree(cpuModel);
             cpuModel = cpuCandidate;
             cpuData = candidate->data;
+            if (!preferred)
+                break;
         } else {
             virCPUDefFree(cpuCandidate);
         }

In my situation´╝Ü
 virQEMUCapsInitCPU ->cpuDecode  ->x86Decode
    if (!(data = cpuNodeData(arch))
        || cpuDecode(cpu, data, NULL, 0, NULL) < 0)
        goto cleanup;
preferred is always NULL. So we can do it better.

> Martin


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]