>-----Original Message-----
>From: Avi Kivity [mailto:[EMAIL PROTECTED]
>Sent: 2007年8月2日 19:58
>To: He, Qing
>Cc: kvm-devel
>Subject: Re: [kvm-devel] [RFC] lapic3: cleanup for save/restore data structure
>of
>in-kernel irqchips
>
>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.
Well, I don't think we can safely presume the code will never change. For
example, maybe someone wants to make IOAPIC redir table count dynamically
configurable, or consolidate IOSAPIC support into current IOAPIC code, etc. In
these cases, the duplicated data structure is capable to become a trap.
Furthermore, even if there is no maintenance concern, these codes are
still not elegant enough.
>- 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 don't see your concern here. The whole lapic3 is work in progress, isn't it?
Why should we maintain an `old' interface? Also, as in the second patch, no
userspace change is required.
Anyway, as the name, it's just cleanup on coding style. It is acceptable to me
to remain as it is.
>
>> 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/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
