From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MTfBb-0004fW-6q for qemu-devel@nongnu.org; Wed, 22 Jul 2009 13:00:59 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MTfBW-0004ZR-JO for qemu-devel@nongnu.org; Wed, 22 Jul 2009 13:00:58 -0400 Received: from [199.232.76.173] (port=59538 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MTfBW-0004Yv-B5 for qemu-devel@nongnu.org; Wed, 22 Jul 2009 13:00:54 -0400 Received: from mx20.gnu.org ([199.232.41.8]:44859) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1MTfBV-0006SI-GK for qemu-devel@nongnu.org; Wed, 22 Jul 2009 13:00:53 -0400 Received: from lizzard.sbs.de ([194.138.37.39]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MTfBU-0004xm-PT for qemu-devel@nongnu.org; Wed, 22 Jul 2009 13:00:53 -0400 Message-ID: <4A6745C0.5060201@siemens.com> Date: Wed, 22 Jul 2009 19:00:48 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <200907221626.n6MGQGH0022104@d03av02.boulder.ibm.com> In-Reply-To: <200907221626.n6MGQGH0022104@d03av02.boulder.ibm.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [COMMIT bd83677] Fake dirty loggin when it's not there List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: qemu-devel , Alexander Graf Anthony Liguori wrote: > From: Alexander Graf > > Some KVM platforms don't support dirty logging yet, like IA64 and PPC, > so in order to still have screen updates on those, we need to fake it. > > This patch just tells the getter function for dirty bitmaps, that all > pages within a slot are dirty when the slot has dirty logging enabled. > > That way we can implement dirty logging on those platforms sometime when > it drags down performance, but share the rest of the code with dirty > logging capable platforms. Sorry to disturb the merge, but my remarks remained unpatched. Would be nice if they are considered in some follow-up patch... :) > > Signed-off-by: Alexander Graf > Signed-off-by: Anthony Liguori > > diff --git a/kvm-all.c b/kvm-all.c > index 961fa32..2032949 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -300,6 +300,7 @@ int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, > KVMDirtyLog d; > KVMSlot *mem; > int ret = 0; > + int r; > > d.dirty_bitmap = NULL; > while (start_addr < end_addr) { > @@ -308,6 +309,11 @@ int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, > break; > } > > + /* We didn't activate dirty logging? Don't care then. */ > + if(!(mem->flags & KVM_MEM_LOG_DIRTY_PAGES)) { > + continue; > + } > + According to Alex' reply [1], I think this is probably better expressed as an assert() than a silent skip. It indicates an error at higher level, no? > size = ((mem->memory_size >> TARGET_PAGE_BITS) + 7) / 8; > if (!d.dirty_bitmap) { > d.dirty_bitmap = qemu_malloc(size); > @@ -319,7 +325,8 @@ int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, > > d.slot = mem->slot; > > - if (kvm_vm_ioctl(s, KVM_GET_DIRTY_LOG, &d) == -1) { > + r = kvm_vm_ioctl(s, KVM_GET_DIRTY_LOG, &d); > + if (r == -EINVAL) { > dprintf("ioctl failed %d\n", errno); > ret = -1; > break; My remark still stands: This is an "optimistic" assumption that only EINVAL is a "real" error. If KVM is buggy /wrt a specific invalid return value, work around that particular bug (and we should also fix the kernel, but that's a different topic). Jan [1] http://permalink.gmane.org/gmane.comp.emulators.qemu/47996