On Fri, Nov 02, 2018 at 03:30:56PM +0100, Michal Privoznik wrote: > 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@xxxxxxxxxx> > > --- > > > > 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 Yep, I pulled it completely apart again only to confirm that we have to go with ^this suggestion, I'll send a v2 in a minute. Erik > node_device_conf.c or you move it into src/util/ because conf module can > use util. > > The rest looks good. > > Michal > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list