[libvirt] [RFC PATCH 0/6] Split domain_conf.h

Ján Tomko posted 6 patches 4 years, 8 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/cover.1563538286.git.jtomko@redhat.com
src/access/viraccessdriver.h              |    2 +-
src/access/viraccessmanager.h             |    2 +-
src/conf/Makefile.inc.am                  |    3 +
src/conf/domain_conf.h                    | 3023 +--------------------
src/conf/domain_format.h                  |   89 +
src/conf/domain_parse.h                   |  321 +++
src/conf/virchrdev.h                      |    2 +-
src/conf/virconftypes.h                   |    3 +
src/conf/virdomaintypes.h                 | 2692 ++++++++++++++++++
src/conf/virnetworkportdef.h              |    4 +-
src/esx/esx_private.h                     |    2 +-
src/interface/interface_backend_udev.c    |    2 +-
src/libxl/libxl_capabilities.c            |    2 +-
src/libxl/libxl_domain.h                  |    2 +-
src/locking/domain_lock.h                 |    2 +-
src/locking/lock_driver.h                 |    2 +-
src/locking/sanlock_helper.c              |    2 +-
src/lxc/lxc_cgroup.h                      |    2 +-
src/lxc/lxc_conf.h                        |    2 +-
src/lxc/lxc_hostdev.h                     |    2 +-
src/lxc/lxc_monitor.h                     |    2 +-
src/lxc/lxc_native.c                      |    2 +-
src/lxc/lxc_native.h                      |    2 +-
src/network/bridge_driver.h               |    2 +-
src/nwfilter/nwfilter_dhcpsnoop.c         |    2 +-
src/nwfilter/nwfilter_driver.c            |    2 +-
src/nwfilter/nwfilter_ebiptables_driver.c |    2 +-
src/nwfilter/nwfilter_gentech_driver.c    |    2 +-
src/nwfilter/nwfilter_learnipaddr.c       |    2 +-
src/qemu/qemu_alias.h                     |    2 +-
src/qemu/qemu_blockjob.c                  |    2 +-
src/qemu/qemu_capabilities.c              |    2 +-
src/qemu/qemu_cgroup.h                    |    2 +-
src/qemu/qemu_command.c                   |    2 +-
src/qemu/qemu_command.h                   |    2 +-
src/qemu/qemu_conf.h                      |    2 +-
src/qemu/qemu_domain.h                    |    2 +-
src/qemu/qemu_domain_address.h            |    2 +-
src/qemu/qemu_driver.c                    |    2 +-
src/qemu/qemu_firmware.h                  |    2 +-
src/qemu/qemu_hostdev.h                   |    2 +-
src/qemu/qemu_hotplug.h                   |    2 +-
src/qemu/qemu_interface.h                 |    2 +-
src/qemu/qemu_monitor.h                   |    2 +-
src/qemu/qemu_processpriv.h               |    2 +-
src/qemu/qemu_security.h                  |    2 +-
src/qemu/qemu_tpm.c                       |    2 +-
src/remote/remote_daemon_dispatch.c       |    2 +-
src/security/security_manager.h           |    2 +-
src/security/virt-aa-helper.c             |    2 +-
src/storage/storage_driver.h              |    2 +-
src/test/test_driver.c                    |    2 +-
src/vbox/vbox_driver.c                    |    2 +-
src/vbox/vbox_network.c                   |    2 +-
src/vbox/vbox_storage.c                   |    2 +-
src/vbox/vbox_tmpl.c                      |    2 +-
src/vz/vz_utils.h                         |    2 +-
src/xenapi/xenapi_driver.c                |    2 +-
src/xenapi/xenapi_utils.c                 |    2 +-
src/xenapi/xenapi_utils.h                 |    2 +-
src/xenconfig/xen_common.h                |    2 +-
src/xenconfig/xen_xl.c                    |    2 +-
src/xenconfig/xen_xl.h                    |    2 +-
src/xenconfig/xen_xm.h                    |    2 +-
tests/domainconftest.c                    |    2 +-
tests/qemumemlocktest.c                   |    2 +-
tests/qemumonitortestutils.h              |    2 +-
tests/qemusecuritytest.c                  |    2 +-
tests/testutilslxc.c                      |    2 +-
tests/testutilsqemu.h                     |    2 +-
tests/testutilsxen.c                      |    2 +-
tools/virsh-domain.c                      |    2 +-
tools/virsh.c                             |    3 -
tools/vsh.c                               |    3 -
74 files changed, 3177 insertions(+), 3094 deletions(-)
create mode 100644 src/conf/domain_format.h
create mode 100644 src/conf/domain_parse.h
create mode 100644 src/conf/virdomaintypes.h
[libvirt] [RFC PATCH 0/6] Split domain_conf.h
Posted by Ján Tomko 4 years, 8 months ago
Currently, domain_conf.{c,h} is a giant pile of functions somewhat
related to the domain definition. Try to change that by splitting out
the type declarations, XML parsing and XML formatting from the header
file.

Ján Tomko (6):
  conf: move virNetworkPortDefPtr declaration to virconftypes.h
  virsh: clean up includes
  conf: introduce virdomaintypes.h
  conf: introduce domain_parse.h
  conf: introduce domain_format.h
  maint: include virdomaintypes.h instead of domain_conf.h

 src/access/viraccessdriver.h              |    2 +-
 src/access/viraccessmanager.h             |    2 +-
 src/conf/Makefile.inc.am                  |    3 +
 src/conf/domain_conf.h                    | 3023 +--------------------
 src/conf/domain_format.h                  |   89 +
 src/conf/domain_parse.h                   |  321 +++
 src/conf/virchrdev.h                      |    2 +-
 src/conf/virconftypes.h                   |    3 +
 src/conf/virdomaintypes.h                 | 2692 ++++++++++++++++++
 src/conf/virnetworkportdef.h              |    4 +-
 src/esx/esx_private.h                     |    2 +-
 src/interface/interface_backend_udev.c    |    2 +-
 src/libxl/libxl_capabilities.c            |    2 +-
 src/libxl/libxl_domain.h                  |    2 +-
 src/locking/domain_lock.h                 |    2 +-
 src/locking/lock_driver.h                 |    2 +-
 src/locking/sanlock_helper.c              |    2 +-
 src/lxc/lxc_cgroup.h                      |    2 +-
 src/lxc/lxc_conf.h                        |    2 +-
 src/lxc/lxc_hostdev.h                     |    2 +-
 src/lxc/lxc_monitor.h                     |    2 +-
 src/lxc/lxc_native.c                      |    2 +-
 src/lxc/lxc_native.h                      |    2 +-
 src/network/bridge_driver.h               |    2 +-
 src/nwfilter/nwfilter_dhcpsnoop.c         |    2 +-
 src/nwfilter/nwfilter_driver.c            |    2 +-
 src/nwfilter/nwfilter_ebiptables_driver.c |    2 +-
 src/nwfilter/nwfilter_gentech_driver.c    |    2 +-
 src/nwfilter/nwfilter_learnipaddr.c       |    2 +-
 src/qemu/qemu_alias.h                     |    2 +-
 src/qemu/qemu_blockjob.c                  |    2 +-
 src/qemu/qemu_capabilities.c              |    2 +-
 src/qemu/qemu_cgroup.h                    |    2 +-
 src/qemu/qemu_command.c                   |    2 +-
 src/qemu/qemu_command.h                   |    2 +-
 src/qemu/qemu_conf.h                      |    2 +-
 src/qemu/qemu_domain.h                    |    2 +-
 src/qemu/qemu_domain_address.h            |    2 +-
 src/qemu/qemu_driver.c                    |    2 +-
 src/qemu/qemu_firmware.h                  |    2 +-
 src/qemu/qemu_hostdev.h                   |    2 +-
 src/qemu/qemu_hotplug.h                   |    2 +-
 src/qemu/qemu_interface.h                 |    2 +-
 src/qemu/qemu_monitor.h                   |    2 +-
 src/qemu/qemu_processpriv.h               |    2 +-
 src/qemu/qemu_security.h                  |    2 +-
 src/qemu/qemu_tpm.c                       |    2 +-
 src/remote/remote_daemon_dispatch.c       |    2 +-
 src/security/security_manager.h           |    2 +-
 src/security/virt-aa-helper.c             |    2 +-
 src/storage/storage_driver.h              |    2 +-
 src/test/test_driver.c                    |    2 +-
 src/vbox/vbox_driver.c                    |    2 +-
 src/vbox/vbox_network.c                   |    2 +-
 src/vbox/vbox_storage.c                   |    2 +-
 src/vbox/vbox_tmpl.c                      |    2 +-
 src/vz/vz_utils.h                         |    2 +-
 src/xenapi/xenapi_driver.c                |    2 +-
 src/xenapi/xenapi_utils.c                 |    2 +-
 src/xenapi/xenapi_utils.h                 |    2 +-
 src/xenconfig/xen_common.h                |    2 +-
 src/xenconfig/xen_xl.c                    |    2 +-
 src/xenconfig/xen_xl.h                    |    2 +-
 src/xenconfig/xen_xm.h                    |    2 +-
 tests/domainconftest.c                    |    2 +-
 tests/qemumemlocktest.c                   |    2 +-
 tests/qemumonitortestutils.h              |    2 +-
 tests/qemusecuritytest.c                  |    2 +-
 tests/testutilslxc.c                      |    2 +-
 tests/testutilsqemu.h                     |    2 +-
 tests/testutilsxen.c                      |    2 +-
 tools/virsh-domain.c                      |    2 +-
 tools/virsh.c                             |    3 -
 tools/vsh.c                               |    3 -
 74 files changed, 3177 insertions(+), 3094 deletions(-)
 create mode 100644 src/conf/domain_format.h
 create mode 100644 src/conf/domain_parse.h
 create mode 100644 src/conf/virdomaintypes.h

-- 
2.19.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH 0/6] Split domain_conf.h
Posted by Daniel P. Berrangé 4 years, 8 months ago
On Fri, Jul 19, 2019 at 02:15:42PM +0200, Ján Tomko wrote:
> Currently, domain_conf.{c,h} is a giant pile of functions somewhat
> related to the domain definition. Try to change that by splitting out
> the type declarations, XML parsing and XML formatting from the header
> file.

Refactoring this is totally overdue.

For most newer stuff we've been using a different naming
convention and split of code, more closely following the
1 file per object / def, named to match. I think it'd be good
to align with that more closely.

I can see that the virdomaindef.c file is still going to
be quite huge though. So splitting off the parse + format
code would still be a win, at least for the virDomainDef.
Probably not worth it for virDomainObj.

I'm not convinced we need to have a separate header
just for the typedefs, without the helper methds.

So how about, as a starting point:

     virdomaindef.h
     virdomaindef.c
     virdomaindefparse.h
     virdomaindefparse.c
     virdomaindefformat.h
     virdomaindefformat.c
     virdomainobj.h
     virdomainobj.c


Some of the stuff we currently have in domain_conf.c
is really stuff that belongs in the virt drivers, but
we dumped it into domain_conf.c so that we could share
it across drivers.

This could suggest a virdomain{obj,def}helpers.{c.h}
for the virt driver code that's being shared.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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