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

Re: [libvirt] [PATCH 12/21] resctrl: Instantiate all resctrl information at once



On Mon, Nov 13, 2017 at 09:50:27AM +0100, Martin Kletzander wrote:
> This allows for looking up the cache control information more sensibly from
> conf/capabilities.c and also provides more data to the virresctrl module that
> will get more usable later on.
> 
> Signed-off-by: Martin Kletzander <mkletzan redhat com>
> ---
>  po/POTFILES.in           |   1 +
>  src/conf/capabilities.c  |  48 +++----
>  src/conf/capabilities.h  |   4 +-
>  src/libvirt_private.syms |   4 +-
>  src/util/virresctrl.c    | 335 +++++++++++++++++++++++++++++++++--------------
>  src/util/virresctrl.h    |  24 ++--
>  6 files changed, 274 insertions(+), 142 deletions(-)

In addition to Jonh's review I would split this patch into 3 separate
patches:

    - one is the simple rename of struct _virResctrlInfo to
      _virResctrlInfoPerCache,

    - second patch would be the introduction of object virResctrlInfo
      (preferable with virResctrlInfoObj or similar name) and helper
      functions to collect that data,

    - the last one would be the switch to collect the resctrl info
      separately and use it to update cache information.

This one patch itself is really hard to properly review.  This is what
usually happens if you have the code working and you need to split it
into patches :).

Otherwise it looks good.

Pavel

Attachment: signature.asc
Description: PGP signature


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