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@xxxxxxxxxx> > > --- > > > > 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list