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

Re: [libvirt] [PATCH RFC 3/9] nodedev: Convert virNodeDevObj[List] to use virPoolObj[Table]



On 02/11/2017 05:29 PM, John Ferlan wrote:
> Use the virPoolObj[Table] object management model in order to manage the
> Node Device objects.
> 
> While making the adjustments to use the new model, there are some code
> formatting adjustments that were also made with the goal to follow more
> recent code flow and layout.
> 
> For API's that were static, rather than use "virNode*", some names were
> converted to be "node*" - makes it easier to determine while reading code
> what is local and what is "outside" the perveyance of the module.
> 
> For the node_device_hal.c - removed the usage of privateData - it wasn't
> actually used anyway other than alloc()/free().  The NODE_DEV_UDI was never
> referenced.
> 
> Signed-off-by: John Ferlan <jferlan redhat com>
> ---
>  po/POTFILES.in                       |   1 +
>  src/Makefile.am                      |   3 +-
>  src/conf/node_device_conf.c          | 497 +-----------------------------
>  src/conf/node_device_conf.h          |  85 +-----
>  src/conf/virnodedeviceobj.c          | 570 +++++++++++++++++++++++++++++++++++
>  src/conf/virnodedeviceobj.h          |  86 ++++++
>  src/libvirt_private.syms             |  25 +-
>  src/node_device/node_device_driver.c | 388 ++++++++++--------------
>  src/node_device/node_device_driver.h |   2 +-
>  src/node_device/node_device_hal.c    |  82 ++---
>  src/node_device/node_device_udev.c   |  76 +++--
>  src/test/test_driver.c               | 294 +++++++-----------
>  12 files changed, 1032 insertions(+), 1077 deletions(-)
>  create mode 100644 src/conf/virnodedeviceobj.c
>  create mode 100644 src/conf/virnodedeviceobj.h
> 

I find it very hard to follow this patch. I think three separate things
are happening here: rename of some static functions, moving
virNodeDeviceObjList* functions into a separate file and switching to
new virPoolObj* APIs.

The other patches are same from this POV. So I'm gonna stop my review here.

My overall feeling is good. But there are some issues we need to
resolve. Also, I don't know why it is, but I'd expect the actual code to
shrink in size after this patch set (we are merging some code after
all), but that is not the case. I don't know what part to hold accountable.

Michal


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