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

Re: [Xen-devel] [PATCH v3 04/15] x86: refactor psr: Encapsulate 'cbm_len' and 'cbm_max'



On 16-11-29 02:43:24, Jan Beulich wrote:
> >>> On 29.11.16 at 05:38, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> > To make codes be better reviewable, I propose below three suggestions:
> > 1) I write a design document about refactor to make you more easily 
> > understand the ideas.
> 
> This is appreciated, but I don't think it's strictly necessary. Describing
> the new design (rather than the changes to the existing one) is what
> likely would be more useful (I'm sorry if I've misunderstood what you
> said, and you in fact had meant just this), which iirc you already have
> in patch 1.
> 
> > 2) Replace the psr.c with a new file which discards all old codes so
> > that you do not need to consider old implementations which may cause
> > confusion.
> > 3) Discard the refactor codes. Just implement L2 CAT based on current
> > codes. Because L2 CAT does not have much difference with L3.
> 
> I don't think introducing a new file is the ideal approach. I'd suggest
> to rip out the entire implementation in a first patch, leaving just
> empty functions to avoid breaking the build (i.e. perhaps mostly the
> ones used by domctl/sysctl, and maybe some init one). Then
> introduce new code, ideally of course not in one big patch, but
> broken up into logical pieces where possible (one such split would be
> that of course you don't need to re-implement domctl/sysctl handling
> in the same patch as everything else).
> 
> Jan
> 
Thanks for your suggestion! Just want to confirm if my understanding is
right. So, you mean I can remove all old codes but keep the interfaces as
empty functions to make sure the build can pass. Then, implement whole
functionality step by step. Right?

Thanks,
Sun Yi

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.