diff mbox series

[v2,03/19] accel/kvm: Convert to ram_block_discard_disable()

Message ID 20200520123152.60527-4-david@redhat.com (mailing list archive)
State New, archived
Headers show
Series virtio-mem: Paravirtualized memory hot(un)plug | expand

Commit Message

David Hildenbrand May 20, 2020, 12:31 p.m. UTC
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(-)

Comments

Philippe Mathieu-Daudé May 20, 2020, 1 p.m. UTC | #1
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;
>
David Hildenbrand May 20, 2020, 1:01 p.m. UTC | #2
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 mbox series

Patch

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;