This is a different take on an RFE I posted in July:
https://www.redhat.com/archives/libvir-list/2018-July/msg01815.html
The goal of this series is to get us close to being able to
raise errors from ToString and FromString functions directly. This
will allow us to drop many hundreds of lines of error reporting
and accompanying translator load.
The first attempt above added a new macro, VIR_ENUM_IMPL_LABEL,
which would take a string label, like 'domain type' for virDomainType
enum. The generated ToString and FromString functions for that
enum would now raise errors containing the label. Redundant error
reporting could now be stripped out piece by piece as individual
enums are converted.
Thinking some more though I don't really like this strategy. Calling
code won't have an easy way to know whether ToString/FromString
functions are the type that raise errors or not, which could lead
to new code accidentally omitting them and failing to raise an error.
It could also stretch out the time that the code base is in a half
transitioned state.
This series is a slightly different direction. VIR_ENUM_IMPL
now always takes a label, but for now we don't actually raise any
errors. Later at a chosen time we can enable error reporting in the
code and begin removing duplicate error reporting. The only downside
is that until the transition is complete, there may be cases of
double error reporting, but that's the lesser of two evils compared
to the previous approach IMO.
So the feedback I'm looking for:
- Is this worth doing? (danpb+mprivozn said as much for first posting)
- Is this approach acceptable?
- Review of the strings added in the last patch
Notes:
- This is on top of pkrempa's virenum.c patches on list
- The last two patches are separated to make review easier, but they
will be committed together
- When enabled, one notable case this will make error reporting worse
is virTristate usage. But I think if we manually implement
*TOString/FromString wrappers in that case we can require callers
to pass in an identifying string. Something to think about for later
Cole Robinson (4):
Always put _LAST enums on second line of VIR_ENUM_IMPL
cfg.mk: Only force _LAST enum on VIR_ENUM_IMPL second line
util: Add 'label' field to VIR_ENUM_IMPL
Add string labels to all VIR_ENUM_IMPL calls
cfg.mk | 6 +-
docs/apibuild.py | 12 ++
src/access/viraccessperm.c | 20 +-
src/conf/capabilities.c | 3 +-
src/conf/cpu_conf.c | 21 +-
src/conf/device_conf.c | 3 +-
src/conf/domain_capabilities.c | 3 +-
src/conf/domain_conf.c | 333 +++++++++++++++++++-----------
src/conf/interface_conf.c | 2 +-
src/conf/netdev_vlan_conf.c | 3 +-
src/conf/network_conf.c | 11 +-
src/conf/node_device_conf.c | 12 +-
src/conf/numa_conf.c | 7 +-
src/conf/nwfilter_conf.c | 21 +-
src/conf/snapshot_conf.c | 6 +-
src/conf/storage_adapter_conf.c | 2 +-
src/conf/storage_conf.c | 18 +-
src/libxl/libxl_domain.c | 3 +-
src/locking/lock_daemon.c | 3 +-
src/logging/log_daemon.c | 3 +-
src/lxc/lxc_domain.c | 7 +-
src/lxc/lxc_native.c | 3 +-
src/network/leaseshelper.c | 3 +-
src/qemu/qemu_agent.c | 4 +-
src/qemu/qemu_capabilities.c | 3 +-
src/qemu/qemu_command.c | 27 ++-
src/qemu/qemu_domain.c | 12 +-
src/qemu/qemu_driver.c | 6 +-
src/qemu/qemu_firmware.c | 6 +-
src/qemu/qemu_migration.c | 3 +-
src/qemu/qemu_migration_cookie.c | 2 +-
src/qemu/qemu_migration_params.c | 9 +-
src/qemu/qemu_monitor.c | 8 +-
src/qemu/qemu_monitor_json.c | 10 +-
src/remote/remote_daemon.c | 3 +-
src/util/vircgroup.c | 3 +-
src/util/vircgroupbackend.c | 3 +-
src/util/vircgroupv1.c | 3 +-
src/util/vircgroupv2.c | 3 +-
src/util/virconf.c | 3 +-
src/util/virenum.c | 28 ++-
src/util/virenum.h | 15 +-
src/util/virerror.c | 3 +-
src/util/virfirewall.c | 3 +-
src/util/virfirewalld.c | 6 +-
src/util/virgic.c | 3 +-
src/util/virhook.c | 20 +-
src/util/virkeycode.c | 3 +-
src/util/virlog.c | 3 +-
src/util/virmdev.c | 3 +-
src/util/virnetdev.c | 6 +-
src/util/virnetdevmacvlan.c | 3 +-
src/util/virnetdevvportprofile.c | 6 +-
src/util/virpci.c | 9 +-
src/util/virperf.c | 3 +-
src/util/virprocess.c | 3 +-
src/util/virresctrl.c | 12 +-
src/util/virsecret.c | 3 +-
src/util/virstorageencryption.c | 4 +-
src/util/virstoragefile.c | 17 +-
src/util/virsysinfo.c | 3 +-
src/util/virtypedparam.c | 3 +-
src/util/virutil.c | 1 -
src/vmware/vmware_conf.c | 3 +-
src/vmx/vmx.c | 3 +-
tests/group-qemu-caps.pl | 2 +-
tests/virkeycodetest.c | 3 +-
tools/virsh-domain-monitor.c | 24 +--
tools/virsh-domain.c | 56 ++---
tools/virsh-host.c | 3 +-
tools/virsh-network.c | 10 +-
tools/virsh-nodedev.c | 2 +-
tools/virsh-pool.c | 4 +-
tools/virsh-secret.c | 2 +-
tools/virsh-volume.c | 5 +-
tools/virt-admin.c | 2 +-
tools/virt-host-validate-common.c | 3 +-
77 files changed, 561 insertions(+), 334 deletions(-)
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list