On 11/02/2009 06:20 PM, Jan Kiszka wrote:
Add a new IOCTL pair to retrieve or set the VCPU state in one chunk.
More precisely, the IOCTL is able to process a list of substates to be
read or written. This list is easily extensible without breaking the
existing ABI, thus we will no longer have to add new IOCTLs when we
discover a missing VCPU state field or want to support new hardware
features.
I'm having some second thoughts about this.
What does the new API buy us? Instead of declaring two new ioctls for
new/fixed substates, we only have to declare one. We still have the
capability check. We still have to declare a structure.
It's true that the internals are currently a mess. We can fix that with
a table-driven approach:
static struct kvm_state_ioctl state_ioctls[] = {
{
.get_ioctl = KVM_GET_FPU,
.set_ioctl = KVM_SET_FPU,
.get = kvm_get_fpu,
.set = kvm_set_fpu,
.size = sizeof(struct kvm_fpu), /* 0 for variable-size state */
},
};
(btw, the new stuff would also benefit from this).
So, what's the real justification for the new ABI?
Jan, my apologies for raising this at such a very late stage in the
review, after all the nits have been satisfactorily addressed. But I
want to make sure we don't bloat the interface without very good reasons.
--
error compiling committee.c: too many arguments to function
--
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
