Re: [PATCH RFC v2 6/6] KVM: introduce a new API for getting dirty bitmaps

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

 



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.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux