diff mbox

Properly check if 'log dirty' flags have changed

Message ID 1392644839-20887-2-git-send-email-vincent.kherbache@inria.fr
State New
Headers show

Commit Message

Vincent KHERBACHE Feb. 17, 2014, 1:47 p.m. UTC
The test (!!(mem->flags & KVM_MEM_LOG_DIRTY_PAGES) == enable) is not
good because the condition is valid when enable == 0 and current dirty log
memory flag is set.
As a consequence kvm_log_global_stop() does not stop the KVM dirty log
tracking: kvm_set_migration_log(0) didn't do its job.
So instead I propose to use kvm_slot_dirty_pages_log_change() which correctly
compare the memory flags (old/new).

Signed-off-by: Vincent KHERBACHE <vincent.kherbache@inria.fr>
---
 kvm-all.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

Comments

Eric Blake Feb. 17, 2014, 5:23 p.m. UTC | #1
On 02/17/2014 06:47 AM, Vincent KHERBACHE wrote:
> The test (!!(mem->flags & KVM_MEM_LOG_DIRTY_PAGES) == enable) is not
> good because the condition is valid when enable == 0 and current dirty log
> memory flag is set.
> As a consequence kvm_log_global_stop() does not stop the KVM dirty log
> tracking: kvm_set_migration_log(0) didn't do its job.
> So instead I propose to use kvm_slot_dirty_pages_log_change() which correctly
> compare the memory flags (old/new).
> 
> Signed-off-by: Vincent KHERBACHE <vincent.kherbache@inria.fr>
> ---
>  kvm-all.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 

> +        err = kvm_slot_dirty_pages_log_change(mem, (bool)enable);

Casting to bool looks odd.  We already require a compliant C99 compiler,
which means the compiler already properly handles the squashing of all
non-zero values to true when calling a function with a parameter
prototyped as bool, without needing the cast.
diff mbox

Patch

diff --git a/kvm-all.c b/kvm-all.c
index 2ca9143..f104f87 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -355,7 +355,7 @@  static int kvm_set_migration_log(int enable)
 {
     KVMState *s = kvm_state;
     KVMSlot *mem;
-    int i, err;
+    int i, err = 0;
 
     s->migration_log = enable;
 
@@ -365,15 +365,9 @@  static int kvm_set_migration_log(int enable)
         if (!mem->memory_size) {
             continue;
         }
-        if (!!(mem->flags & KVM_MEM_LOG_DIRTY_PAGES) == enable) {
-            continue;
-        }
-        err = kvm_set_user_memory_region(s, mem);
-        if (err) {
-            return err;
-        }
+        err = kvm_slot_dirty_pages_log_change(mem, (bool)enable);
     }
-    return 0;
+    return err;
 }
 
 /* get kvm's dirty pages bitmap and update qemu's */