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

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

 



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@xxxxxxxxxx>
> ---
>  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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux