Re: [RFC] lapic3: cleanup for save/restore data structure of in-kernel irqchips

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

He, Qing wrote:
> Hi,
> 	The argument of in-kernel irqchip save/restore IOCTL uses a
> separate data structure (struct kvm_irqchip and struct kvm_ioctl_pic in
> include/linux/kvm.h) different from functional data structure (struct
> kvm_pic_state and struct kvm_ioapic in driver/kvm/irq.h), this is
> because while most data are used at both side, there are certain
> internal data in functional data structure which should not be exposed
> to the user.
> 	Current code has some duplication between these two, and these
> duplicated parts are copied back and forth when save and restore. This
> seems a maintenance pain.
>   

I'm not sure consolidating helps with maintenance in this case:

- if there are no changes, there is no maintenance.
- if we change the kernel side, we can't change the userspace interface 
side because it will break existing code.  we have to write a new data 
structure for the interface and maintain the new and the old in parallel.

> 	I have come to two different possible solutions, but there are
> some coding style concerns:
> 	1. use macro definitions to sort out the common part,. The
> problem is that this is not so elegant and I hardly find anything
> similar in kernel code.
> 	2. organize the common part into a new struct, and include this
> struct into ioctl argument and functional data structure. The problem of
> this approach is that we have to re-write many codes to make things like
> `s->irr' changing to `s->state.irr'.
>
>   

I guess the later is preferable.  But I'm not sure it's worthwhile due 
to the issue above.



-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/