| Message ID | 20200520123152.60527-4-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | virtio-mem: Paravirtualized memory hot(un)plug | expand |
Hi David, On 5/20/20 2:31 PM, David Hildenbrand wrote: > Discarding memory does not work as expected. At the time this is called, > we cannot have anyone active that relies on discards to work properly. > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > accel/kvm/kvm-all.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index d06cc04079..7a6158fb99 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -40,7 +40,6 @@ > #include "trace.h" > #include "hw/irq.h" > #include "sysemu/sev.h" > -#include "sysemu/balloon.h" > #include "qapi/visitor.h" > #include "qapi/qapi-types-common.h" > #include "qapi/qapi-visit-common.h" > @@ -2143,7 +2142,7 @@ static int kvm_init(MachineState *ms) > > s->sync_mmu = !!kvm_vm_check_extension(kvm_state, KVM_CAP_SYNC_MMU); > if (!s->sync_mmu) { > - qemu_balloon_inhibit(true); > + g_assert(ram_block_discard_disable(true)); Please do not evaluate code within an assert() call. See the comment added to "qemu/osdep.h" in commit 262a69f4282: /* * We have a lot of unaudited code that may fail in strange ways, or * even be a security risk during migration, if you disable assertions * at compile-time. You may comment out these safety checks if you * absolutely want to disable assertion overhead, but it is not * supported upstream so the risk is all yours. Meanwhile, please * submit patches to remove any side-effects inside an assertion, or * fixing error handling that should use Error instead of assert. */ > } > > return 0; >
On 20.05.20 15:00, Philippe Mathieu-Daudé wrote: > Hi David, > > On 5/20/20 2:31 PM, David Hildenbrand wrote: >> Discarding memory does not work as expected. At the time this is called, >> we cannot have anyone active that relies on discards to work properly. >> >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> accel/kvm/kvm-all.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c >> index d06cc04079..7a6158fb99 100644 >> --- a/accel/kvm/kvm-all.c >> +++ b/accel/kvm/kvm-all.c >> @@ -40,7 +40,6 @@ >> #include "trace.h" >> #include "hw/irq.h" >> #include "sysemu/sev.h" >> -#include "sysemu/balloon.h" >> #include "qapi/visitor.h" >> #include "qapi/qapi-types-common.h" >> #include "qapi/qapi-visit-common.h" >> @@ -2143,7 +2142,7 @@ static int kvm_init(MachineState *ms) >> >> s->sync_mmu = !!kvm_vm_check_extension(kvm_state, KVM_CAP_SYNC_MMU); >> if (!s->sync_mmu) { >> - qemu_balloon_inhibit(true); >> + g_assert(ram_block_discard_disable(true)); > > Please do not evaluate code within an assert() call. > Right, thanks for pointing that out.
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index d06cc04079..7a6158fb99 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -40,7 +40,6 @@ #include "trace.h" #include "hw/irq.h" #include "sysemu/sev.h" -#include "sysemu/balloon.h" #include "qapi/visitor.h" #include "qapi/qapi-types-common.h" #include "qapi/qapi-visit-common.h" @@ -2143,7 +2142,7 @@ static int kvm_init(MachineState *ms) s->sync_mmu = !!kvm_vm_check_extension(kvm_state, KVM_CAP_SYNC_MMU); if (!s->sync_mmu) { - qemu_balloon_inhibit(true); + g_assert(ram_block_discard_disable(true)); } return 0;