qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 5/6] Introduce kvm logging interface.


From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH 5/6] Introduce kvm logging interface.
Date: Wed, 19 Nov 2008 09:18:43 -0600
User-agent: Thunderbird 2.0.0.17 (X11/20080925)

Glauber Costa wrote:
Introduce functions to stop and start logging of memory regions.
We select region based on its start address.

Signed-off-by: Glauber Costa <address@hidden>
---
 kvm-all.c |  153 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 kvm.h     |    5 ++
 2 files changed, 158 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 9700e50..c522205 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -32,8 +32,13 @@
     do { } while (0)
 #endif
+#define warning(fmt, ...) \
+    do { dprintf("%s:%d" fmt, __func__, __LINE__, ## __VA_ARGS__); } while (0)

I don't think this macro really serves a purpose. You can just add "%s:%d: " to the beginning of dprintf() if you want.

Also note that it gives goofy output right now because of the missing space after %d.

+/*
+ * dirty pages logging control
+ */
+static int kvm_dirty_pages_log_change(KVMSlot *mem,
+                                      unsigned flags,
+                                      unsigned mask)
+{
+    int r = -1;
+    KVMState *s = kvm_state;
+
+    flags = (mem->flags & ~mask) | flags;
+    if (flags == mem->flags)
+            return 0;
+
+    mem->flags = flags;
+
+    r = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, mem);
+    if (r == -1)
+        fprintf(stderr, "%s: %m\n", __FUNCTION__);
+
+    return r;
+}
+
+int kvm_log_start(target_phys_addr_t phys_addr, target_phys_addr_t end_addr)
+{
+        KVMState *s = kvm_state;
+        KVMSlot *mem = kvm_lookup_slot(s, phys_addr);
+
+        /*
+         * We don't need to do dirty tracking of alias slots. If we track the
+         * memory the alias is pointing to, it should be enough
+         */
+        if (kvm_arch_is_alias_slot(phys_addr))
+            return 0;
+
+        if (mem == NULL)  {
+                warning("invalid parameters %llx-%llx\n", phys_addr, end_addr);
+                return -EINVAL;
+        }
+
+        /* Already logging, no need to issue ioctl */
+        if (mem->flags & KVM_MEM_LOG_DIRTY_PAGES)
+            return 0;
+
+        dprintf("slot %d: enable logging (phys %llx, uaddr %llx)\n",
+                 mem->slot, mem->guest_phys_addr, mem->userspace_addr);
+
+        return kvm_dirty_pages_log_change(mem,
+                                          KVM_MEM_LOG_DIRTY_PAGES,
+                                          KVM_MEM_LOG_DIRTY_PAGES);
+}
+
+int kvm_log_stop(target_phys_addr_t phys_addr, target_phys_addr_t end_addr)
+{
+
+        KVMState *s = kvm_state;
+        KVMSlot *mem = kvm_lookup_slot(s, phys_addr);
+
+        if (kvm_arch_is_alias_slot(phys_addr))
+            return 0;
+
+        if (mem == NULL) {
+                warning("invalid parameters %llx - %llx \n", phys_addr, 
end_addr);
+                return -EINVAL;
+        }
+
+        /* Not logging, no need to issue ioctl */
+        if (!(mem->flags & KVM_MEM_LOG_DIRTY_PAGES))
+            return 0;
+
+        dprintf("slot %d: disabling logging\n", mem->slot);
+        return kvm_dirty_pages_log_change(mem,
+                                          0,
+                                          KVM_MEM_LOG_DIRTY_PAGES);
+}

Note that start and stop are identical except for a different printf(). The call a common helper function. Why not fold everything into kvm_dirty_pages_log_change() and make that the public interface (kvm_set_log).

+static inline int lookup_bitmap_phys(unsigned long *dirty, unsigned nr)
+{
+    unsigned word = nr / (sizeof(*dirty) * 8);
+    unsigned bit = nr % (sizeof(*dirty) * 8);
+    int ret;
+
+    ret = (dirty[word] >> bit) & 1;
+    return ret;
+}
+
+/**
+ * kvm_physical_sync_dirty_bitmap - Grab dirty bitmap from kernel space
+ * If a phys_offset parameter is given, this function updates qemu's dirty
+ * bitmap using cpu_physical_memory_set_dirty(). This means all bits are set
+ * to dirty.
+ *
+ * @start_add: start of logged region. This is what we use to search the 
memslot
+ * @end_addr: end of logged region. Only matters if we are updating qemu dirty 
bitmap.
+ * @phys_offset: the phys_offset we want to use for qemu dirty bitmap update. 
Passing
+ *               NULL makes the update not happen. In this case, we only grab 
the bitmap
+ *               from kernel.
+ */
+void kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, 
target_phys_addr_t end_addr,
+                                    ram_addr_t phys_offset)

This interface is weird and broken. If we wanted to use this for live migration, we would end up passing phys_offset=0 but that has a special meaning here.

But why are we passing phys_offset at all?  Why can't we do the lookup here?

Regards,

Anthony Liguori




reply via email to

[Prev in Thread] Current Thread [Next in Thread]