[libvirt] [python PATCH] build: make it easier to backport event ids

[Jiri Denemark]

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 at 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