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

Daniel P. Berrange berrange at redhat.com
Thu Jan 15 10:42:39 UTC 2015


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 ?

> 
>      include.close()
>      export.close()
> diff --git a/libvirt-override.c b/libvirt-override.c
> index e51c44d..88cb527 100644
> --- a/libvirt-override.c
> +++ b/libvirt-override.c
> @@ -4,7 +4,7 @@
>   *           entry points where an automatically generated stub is
>   *           unpractical
>   *
> - * Copyright (C) 2005, 2007-2014 Red Hat, Inc.
> + * Copyright (C) 2005, 2007-2015 Red Hat, Inc.
>   *
>   * Daniel Veillard <veillard at redhat.com>
>   */
> @@ -6346,7 +6346,7 @@ libvirt_virConnectDomainEventPMSuspendCallback(virConnectPtr conn ATTRIBUTE_UNUS
>  }
> 
> 
> -#if LIBVIR_CHECK_VERSION(0, 10, 0)
> +#ifdef VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE
>  static int
>  libvirt_virConnectDomainEventBalloonChangeCallback(virConnectPtr conn ATTRIBUTE_UNUSED,
>                                                     virDomainPtr dom,
> @@ -6398,9 +6398,9 @@ libvirt_virConnectDomainEventBalloonChangeCallback(virConnectPtr conn ATTRIBUTE_
>      LIBVIRT_RELEASE_THREAD_STATE;
>      return ret;
>  }
> -#endif /* LIBVIR_CHECK_VERSION(0, 10, 0) */
> +#endif /* VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE */
> 
> -#if LIBVIR_CHECK_VERSION(1, 0, 0)
> +#ifdef VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK
>  static int
>  libvirt_virConnectDomainEventPMSuspendDiskCallback(virConnectPtr conn ATTRIBUTE_UNUSED,
>                                                     virDomainPtr dom,
> @@ -6452,9 +6452,9 @@ libvirt_virConnectDomainEventPMSuspendDiskCallback(virConnectPtr conn ATTRIBUTE_
>      LIBVIRT_RELEASE_THREAD_STATE;
>      return ret;
>  }
> -#endif /* LIBVIR_CHECK_VERSION(1, 0, 0) */
> +#endif /* VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK */
> 
> -#if LIBVIR_CHECK_VERSION(1, 1, 1)
> +#ifdef VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED
>  static int
>  libvirt_virConnectDomainEventDeviceRemovedCallback(virConnectPtr conn ATTRIBUTE_UNUSED,
>                                                     virDomainPtr dom,
> @@ -6504,9 +6504,9 @@ libvirt_virConnectDomainEventDeviceRemovedCallback(virConnectPtr conn ATTRIBUTE_
>      LIBVIRT_RELEASE_THREAD_STATE;
>      return ret;
>  }
> -#endif /* LIBVIR_CHECK_VERSION(1, 1, 1) */
> +#endif /* VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED */
> 
> -#if LIBVIR_CHECK_VERSION(1, 2, 9)
> +#ifdef VIR_DOMAIN_EVENT_ID_TUNABLE
>  static int
>  libvirt_virConnectDomainEventTunableCallback(virConnectPtr conn ATTRIBUTE_UNUSED,
>                                               virDomainPtr dom,
> @@ -6564,9 +6564,9 @@ libvirt_virConnectDomainEventTunableCallback(virConnectPtr conn ATTRIBUTE_UNUSED
>      return ret;
> 
>  }
> -#endif /* LIBVIR_CHECK_VERSION(1, 2, 9) */
> +#endif /* VIR_DOMAIN_EVENT_ID_TUNABLE */
> 
> -#if LIBVIR_CHECK_VERSION(1, 2, 11)
> +#ifdef VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE
>  static int
>  libvirt_virConnectDomainEventAgentLifecycleCallback(virConnectPtr conn ATTRIBUTE_UNUSED,
>                                                      virDomainPtr dom,
> @@ -6618,7 +6618,7 @@ libvirt_virConnectDomainEventAgentLifecycleCallback(virConnectPtr conn ATTRIBUTE
>      return ret;
> 
>  }
> -#endif /* LIBVIR_CHECK_VERSION(1, 2, 11) */
> +#endif /* VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE */
> 
> 
>  static PyObject *
> @@ -6676,9 +6676,9 @@ libvirt_virConnectDomainEventRegisterAny(ATTRIBUTE_UNUSED PyObject *self,
>          cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventGenericCallback);
>          break;
>      case VIR_DOMAIN_EVENT_ID_BLOCK_JOB:
> -#if LIBVIR_CHECK_VERSION(1, 2, 6)
> +#ifdef VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2
>      case VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2:
> -#endif /* LIBVIR_CHECK_VERSION(1, 2, 6) */
> +#endif /* VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2 */
>          cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventBlockJobCallback);
>          break;
>      case VIR_DOMAIN_EVENT_ID_DISK_CHANGE:
> @@ -6693,31 +6693,31 @@ libvirt_virConnectDomainEventRegisterAny(ATTRIBUTE_UNUSED PyObject *self,
>      case VIR_DOMAIN_EVENT_ID_PMSUSPEND:
>          cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventPMSuspendCallback);
>          break;
> -#if LIBVIR_CHECK_VERSION(0, 10, 0)
> +#ifdef VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE
>      case VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE:
>          cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventBalloonChangeCallback);
>          break;
> -#endif /* LIBVIR_CHECK_VERSION(0, 10, 0) */
> -#if LIBVIR_CHECK_VERSION(1, 0, 0)
> +#endif /* VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE */
> +#ifdef VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK
>      case VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK:
>          cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventPMSuspendDiskCallback);
>          break;
> -#endif /* LIBVIR_CHECK_VERSION(1, 0, 0) */
> -#if LIBVIR_CHECK_VERSION(1, 1, 1)
> +#endif /* VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK */
> +#ifdef VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED
>      case VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED:
>          cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventDeviceRemovedCallback);
>          break;
> -#endif /* LIBVIR_CHECK_VERSION(1, 1, 1) */
> -#if LIBVIR_CHECK_VERSION(1, 2, 9)
> +#endif /* VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED */
> +#ifdef VIR_DOMAIN_EVENT_ID_TUNABLE
>      case VIR_DOMAIN_EVENT_ID_TUNABLE:
>          cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventTunableCallback);
>          break;
> -#endif /* LIBVIR_CHECK_VERSION(1, 2, 9) */
> -#if LIBVIR_CHECK_VERSION(1, 2, 11)
> +#endif /* VIR_DOMAIN_EVENT_ID_TUNABLE */
> +#ifdef VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE
>      case VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE:
>          cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventAgentLifecycleCallback);
>          break;
> -#endif /* LIBVIR_CHECK_VERSION(1, 2, 11) */
> +#endif /* VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE */
>      case VIR_DOMAIN_EVENT_ID_LAST:
>          break;
>      }
> -- 
> 2.1.0
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|




More information about the libvir-list mailing list