Re: [Qemu-devel] [Qemu-ppc] [PATCH 0/4] spapr:DRC cleanups (part I)

[Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>]

On 06/01/2017 02:30 AM, David Gibson wrote:
> On Wed, May 31, 2017 at 11:25:41PM -0500, Michael Roth wrote:
>> Quoting Bharata B Rao (2017-05-31 23:06:46)
>>> On Thu, Jun 01, 2017 at 11:52:14AM +1000, David Gibson wrote:
>>>> The code managing DRCs[0] has quite a few things that are more
>>>> complicated than they need to be.  In particular the object
>>>> representing a DRC has a bunch of method pointers, despite the fact
>>>> that there are currently no subclasses, and even if there were the
>>>> method implementations would be unlikely to differ.
>>> So you are getting rid of a few methods. How about other methods ?
>>> Specially attach and detach which have incorporated all the logic needed
>>> to handle logical and physical DRs into their implementations ?
>> I would avoid any methods that incorporate special-casing for
>> physical vs. logical DRCs, since that seems like a good logical
>> starting point for moving to 'physical'/'logical' DRC
>> sub-classes to help simplify the increasingly complicated
>> state-tracking.
> Right, I'm looking at making subclasses for each of the DRC types.
> Possibly with intermediate subclasses for physical vs. logical, we'll
> see how it works out.

Back in the DRC migration patch series I talked with Mike about refactoring
the DRC code in such fashion (physical DRC and logical DRC). But first I 
would
implement some kind of unit testing in this code to avoid breaking too much
stuff during this refactoring.

I am not sure about the effort to implementing unit test in the current 
DRC code.
This series is simplifying the DRC code, making it more minimalist and 
possibly
easier to be tested. In the end it would be a first step towards unit 
testing.

However, there is the issue of backward compatibility. I fear this DRC 
refactoring
of Logical/Physical DRC would be too drastic to maintain such compatibility
(assuming that it is not already broken). If this refactor goes live 
only in 2.11 then
we will have a hard time to migrate from 2.11 to 2.10.

All that said, I believe we can live without unit testing for a little 
longer and if
we're going for this Physical/DRC refactoring, we need to push it for 
2.10. We can
think about unit test later with the refactored code. Feel free to send 
to me any
unfinished/beta DRC refactoring code you might be working on and want
tested. I can help in the refactoring too, just let me know.


Daniel


>
>> I also don't think we should expose DRC internal fields to
>> outside callers (which attach/detach would involve).
> Well.. just changing attach/detach to plain functions instead of
> methods wouldn't break that.
>
>> This
>> series does that to some extent with the RTAS calls, but
>> since those are now moved to spapr_drc.c it makes more sense.
> Right - the semantics of the RTAS calls are tied closely to the DRC
> semantics, so I don't think there's any point considering the RTAS
> calls to be "outside" the DRC code itself.
>