On Thu, Jan 15, 2015 at 10:42:39 +0000, Daniel Berrange wrote:
On Wed, Jan 14, 2015 at 08:08:16PM -0700, Eric Blake wrote:
In some cases, it is very easy for downstream distros to backport enum values without requiring a .so bump. Keying the conditional code off of the upstream version where the enum value was added is not ideal, because downstream then has to patch that the feature is available in their build that still reports an earlier version number. For example, if RHEL 7 backports events from 1.2.11 into a build based on 1.2.8, building the python bindings would warn:
libvirt-override.c: In function ‘libvirt_virConnectDomainEventRegisterAny’: libvirt-override.c:6653:5: warning: enumeration value ‘VIR_DOMAIN_EVENT_ID_TUNABLE’ not handled in switch [-Wswitch] switch ((virDomainEventID) eventID) { ^ libvirt-override.c:6653:5: warning: enumeration value ‘VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE’ not handled in switch [-Wswitch]
The solution is simple - use feature-based probes instead of version probes. Since we already scrape the XML API document of whatever libvirt build we are binding, and that XML already documents any downstream enum additions, we can use those as the features for gating conditional compilation.
* generator.py (enum): Track event id names. (buildStubs): Output define wrappers for events. * libvirt-override.c (libvirt_virConnectDomainEventBalloonChangeCallback) (libvirt_virConnectDomainEventPMSuspendDiskCallback) (libvirt_virConnectDomainEventDeviceRemovedCallback) (libvirt_virConnectDomainEventTunableCallback) (libvirt_virConnectDomainEventAgentLifecycleCallback) (libvirt_virConnectDomainEventRegisterAny): Use them.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
I mentioned this idea on the list a while back, and finally got time to code it up in a much nicer way than my initial attempt that would have polluted libvirt.h proper. https://www.redhat.com/archives/libvir-list/2014-October/msg00186.html
I'm relatively weak at python, and welcome to suggestions on how to avoid the long line in generator.py (maybe a regex match on VIR_.*_EVENT_ID_, but what's the canonical way to write that in python?)
generator.py | 11 +++++++++-- libvirt-override.c | 46 +++++++++++++++++++++++----------------------- 2 files changed, 32 insertions(+), 25 deletions(-)
diff --git a/generator.py b/generator.py index cf044c9..4fd6d55 100755 --- a/generator.py +++ b/generator.py @@ -9,6 +9,7 @@ qemu_functions = {} enums = {} # { enumType: { enumConstant: enumValue } } lxc_enums = {} # { enumType: { enumConstant: enumValue } } qemu_enums = {} # { enumType: { enumConstant: enumValue } } +event_ids = []
import os import sys @@ -219,6 +220,8 @@ def lxc_function(name, desc, ret, args, file, module, cond): def enum(type, name, value): if type not in enums: enums[type] = {} + if name.startswith('VIR_DOMAIN_EVENT_ID_') or name.startswith('VIR_NETWORK_EVENT_ID_'): + event_ids.append(name)
If you use brackets you can break the line without confusing python eg
if (name.startswith('VIR_DOMAIN_EVENT_ID_') or name.startswith('VIR_NETWORK_EVENT_ID_')):
if value == 'VIR_TYPED_PARAM_INT': value = 1 elif value == 'VIR_TYPED_PARAM_UINT': @@ -910,10 +913,10 @@ def buildStubs(module, api_xml): wrapper_file = "build/%s.c" % module
include = open(header_file, "w") - include.write("/* Generated */\n\n") + include.write("/* Generated by generator.py */\n\n")
export = open(export_file, "w") - export.write("/* Generated */\n\n") + export.write("/* Generated by generator.py */\n\n")
wrapper = open(wrapper_file, "w") wrapper.write("/* Generated by generator.py */\n\n") @@ -943,6 +946,10 @@ def buildStubs(module, api_xml): # Write C pointer conversion functions. for classname in primary_classes: print_c_pointer(classname, wrapper, export, include) + # Write define wrappers around event id enums + include.write("\n") + for event_id in event_ids: + include.write("#define %s %s\n" % (event_id, event_id))
I'm not seeing the point of adding this define ? ...
-#if LIBVIR_CHECK_VERSION(0, 10, 0) +#ifdef VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE
Without that define, you can't check for the events using ifdef, C preprocessor doesn't see enum items. Jirka