From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH RFC v2 6/6] KVM: introduce a new API for getting dirty bitmaps Date: Fri, 23 Apr 2010 14:58:41 +0300 Message-ID: <4BD18B71.4030202@redhat.com> References: <20100420195349.dab60b1d.yoshikawa.takuya@oss.ntt.co.jp> <20100420200353.2d2a6dec.yoshikawa.takuya@oss.ntt.co.jp> <4BCEE579.9020206@redhat.com> <4BD0181C.6020900@oss.ntt.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: mtosatti-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kvm-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kvm-ppc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, fernando-gVGce1chcLdL9jVzuh4AOg@public.gmane.org To: Takuya Yoshikawa Return-path: In-Reply-To: <4BD0181C.6020900-gVGce1chcLdL9jVzuh4AOg@public.gmane.org> Sender: kvm-ppc-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: kvm.vger.kernel.org On 04/22/2010 12:34 PM, Takuya Yoshikawa wrote: > Thanks, I can know basic rules about kvm/api . > > (2010/04/21 20:46), Avi Kivity wrote: > >> >>> +Type: vm ioctl >>> +Parameters: struct kvm_dirty_log (in/out) >>> +Returns: 0 on success, -1 on error >>> + >>> +/* for KVM_SWITCH_DIRTY_LOG */ >>> +struct kvm_dirty_log { >>> + __u32 slot; >>> + __u32 padding; >> >> Please put a flags field here (and verify it is zero in the ioctl >> implementation). This allows us to extend it later. >> >>> + union { >>> + void __user *dirty_bitmap; /* one bit per page */ >>> + __u64 addr; >>> + }; >> >> Just make dirty_bitmap a __u64. With the union there is the risk that >> someone forgets to zero the structure so we get some random bits in the >> pointer, and also issues with big endian and s390 compat. >> >> Reserve some extra space here for future expansion. >> >> Hm. I see that this is the existing kvm_dirty_log structure. So we can't >> play with it, ignore my comments about it. > > So, introducing a new structure to export(and get) two bitmap addresses > with a flag is the thing? > > 1. Qemu calls ioctl to get the two addresses. > 2. Qemu calls ioctl to make KVM switch the dirty bitmaps. > --> in this case, qemu have to change the address got in step 1. > ... > 3. Qemu calls ioctl(we can reuse 1. with different command flag) to > free the bitmaps. > > > In my plan, we don't let qemu malloc() dirty bitmaps: at least, I want > to make that > another patch set because this patch set already has some dependencies > to other issues. > > But, yes, I can make the struct considering the future expansion if it > needed. I guess it's better, since this patch is a "future expansion" of KVN_GET_DIRTY_LOG. If we had a flags field and some room there, we could keep the old ioctl. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic.