On Thu, 25 Jul 2024 at 13:05, Salil Mehta <salil.mehta@xxxxxxxxxx> wrote: > > HI Peter, > > > From: Peter Maydell <peter.maydell@xxxxxxxxxx> > > Sent: Thursday, July 25, 2024 11:36 AM > > To: Michael S. Tsirkin <mst@xxxxxxxxxx> > > > > On Tue, 23 Jul 2024 at 11:58, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote: > > > > > > From: Salil Mehta <salil.mehta@xxxxxxxxxx> > > > > > > KVM vCPU creation is done once during the vCPU realization when Qemu > > > vCPU thread is spawned. This is common to all the architectures as of now. > > > > > > Hot-unplug of vCPU results in destruction of the vCPU object in QOM > > > but the corresponding KVM vCPU object in the Host KVM is not destroyed > > > as KVM doesn't support vCPU removal. Therefore, its representative KVM > > > vCPU object/context in Qemu is parked. > > > > > > Refactor architecture common logic so that some APIs could be reused > > > by vCPU Hotplug code of some architectures likes ARM, Loongson etc. > > > Update new/old APIs with trace events. New APIs > > > qemu_{create,park,unpark}_vcpu() can be externally called. No functional > > change is intended here. > > > > Hi; Coverity points out an issue with this code (CID 1558552): > > > > > +int kvm_unpark_vcpu(KVMState *s, unsigned long vcpu_id) { > > > + struct KVMParkedVcpu *cpu; > > > + int kvm_fd = -ENOENT; > > > + > > > + QLIST_FOREACH(cpu, &s->kvm_parked_vcpus, node) { > > > + if (cpu->vcpu_id == vcpu_id) { > > > + QLIST_REMOVE(cpu, node); > > > + kvm_fd = cpu->kvm_fd; > > > + g_free(cpu); > > > + } > > > + } > > > > If you are going to remove an entry from a list as you iterate over it, you > > can't use QLIST_FOREACH(), because QLIST_FOREACH will look at the next > > pointer of the iteration variable at the end of the loop when it wants to > > advance to the next node. In this case we've already freed 'cpu', so it would > > be reading freed memory. > > > > Should we break out of the loop when we find the entry? > > > Thanks for identifying this. Yes, a break is missing. Should I send a fix for this > now or you can incorporate it? The code is already in upstream git, so please send a patch to fix the bug. thanks -- PMM