On Thu, Nov 20, 2008 at 01:02:39PM +0200, Avi Kivity wrote:
> Anthony Liguori wrote:
>>
>> That's not how I read the code. I see:
>>
>>>
>>> static void kvm_free_physmem_slot(struct kvm_memory_slot *free,
>>> struct kvm_memory_slot *dont)
>>> {
>>> if (!dont || free->rmap != dont->rmap)
>>> vfree(free->rmap);
>>
>> And it's called as kvm_free_physmem_slot(&old, &new);
>>
>> new is assigned to old to start out with so old.rmap will equal new.rmap.
>>
>
> Hm, if !npages we should just kvm_free_physmem_slot(&old, NULL).
Actually, I believe we need a little bit more than that, because we can
have valid rmaps in flight.
Tell me what you think about this.
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index b1953ee..f605bba 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -735,11 +735,17 @@ int __kvm_set_memory_region(struct kvm *kvm,
base_gfn = mem->guest_phys_addr >> PAGE_SHIFT;
npages = mem->memory_size >> PAGE_SHIFT;
- if (!npages)
- mem->flags &= ~KVM_MEM_LOG_DIRTY_PAGES;
-
new = old = *memslot;
+ if (!npages) {
+ mem->flags &= ~KVM_MEM_LOG_DIRTY_PAGES;
+ kvm_arch_flush_shadow(kvm);
+ kvm_free_physmem_slot(memslot, NULL);
+ kvm_arch_set_memory_region(kvm, mem, old, user_alloc);
+ goto out;
+ }
+
+
new.base_gfn = base_gfn;
new.npages = npages;
new.flags = mem->flags;
@@ -812,9 +818,6 @@ int __kvm_set_memory_region(struct kvm *kvm,
}
#endif /* not defined CONFIG_S390 */
- if (!npages)
- kvm_arch_flush_shadow(kvm);
-
spin_lock(&kvm->mmu_lock);
if (mem->slot >= kvm->nmemslots)
kvm->nmemslots = mem->slot + 1;
