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

Re: [libvirt] [PATCH] nodedev: Add new module node_device_util



On 11/01/2018 04:50 PM, Erik Skultety wrote:
> There's a lot of stuff going on in src/conf/nodedev_conf which not
> always has to do anything with config, so even though we're trying,
> we're not really consistent in putting only parser/formatter related
> stuff here like we do for domains. So, start the cleanup simply by adding
> a new module to the nodedev driver and put a few helper APIs which want
> to open a secondary driver connection in there (similar to storage_util
> module).
> 
> Signed-off-by: Erik Skultety <eskultet redhat com>
> ---
> 
> I verified the build with debian 9, centos 7, fedora 28, rawhide, and freebsd
> 11
> 
>  src/conf/Makefile.inc.am             |   1 +
>  src/conf/node_device_conf.c          | 199 -----------------------
>  src/conf/node_device_conf.h          |  11 --
>  src/conf/virstorageobj.c             |   1 +
>  src/libvirt_private.syms             |   8 +-
>  src/node_device/Makefile.inc.am      |  17 +-
>  src/node_device/node_device_driver.c |   1 +
>  src/node_device/node_device_util.c   | 229 +++++++++++++++++++++++++++
>  src/node_device/node_device_util.h   |  35 ++++
>  src/storage/Makefile.inc.am          |   1 +
>  src/storage/storage_backend_scsi.c   |   1 +
>  11 files changed, 290 insertions(+), 214 deletions(-)
>  create mode 100644 src/node_device/node_device_util.c
>  create mode 100644 src/node_device/node_device_util.h
> 
> diff --git a/src/conf/Makefile.inc.am b/src/conf/Makefile.inc.am
> index af23810640..7cb6c29d70 100644
> --- a/src/conf/Makefile.inc.am
> +++ b/src/conf/Makefile.inc.am
> @@ -163,6 +163,7 @@ libvirt_la_BUILT_LIBADD += libvirt_conf.la
>  libvirt_conf_la_SOURCES = $(CONF_SOURCES)
>  libvirt_conf_la_CFLAGS = \
>  	-I$(srcdir)/conf \
> +	-I$(srcdir)/node_device \

This doesn't feel right. The conf parser/formatter should be driver
agnostic.

I see two options. If you want to clean up src/conf/node_device_conf.c
either you'll put node_device_util.c into src/conf/ right next to
node_device_conf.c or you move it into src/util/ because conf module can
use util.

The rest looks good.

Michal


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