[libvirt] [RFC PATCH] include: make it easier to probe enum growth

[Daniel P. Berrange]

On Mon, Oct 06, 2014 at 02:06:56PM -0600, Eric Blake wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1147639 is an example
> of a downstream distro's dilemma - when backporting a feature that
> is implemented in an ABI-compatible manner (no .so bump was
> required) but where the feature involves new bits to be defined
> in a flags variable, how does one write code to reliably detect
> that those bits have been backported?

My answer would be that distros shouldn't be cherry-picking bits
of the public header file at all so they don't create these
non-standard APIs.


> The solution presented here is a common idiom used in a number of
> other header files (for example, glibc's /usr/include/langinfo.h
> does it for ABDAY_1 and friends); by adding a self-referential
> preprocessor macro, client code can easily do:
> 
> | switch (state) {
> | #ifdef VIR_DOMAIN_PMSUSPENDED
> |   case VIR_DOMAIN_PMSUSPENDED:
> |     ....
> | #endif
> | }
> 
> rather than trying to figure out which version number introduced
> VIR_DOMAIN_PMSUSPENDED (v.9.11), and using that with
> LIBVIR_CHECK_VERSION.  Of course, since 1.2.10 would be the first
> release where this practice is reliable, we will still see clients
> that target earlier libvirt doing:
> 
> | switch (state) {
> | #if LIBVIR_CHECK_VERSION(0, 9, 11) || defined(VIR_DOMAIN_PMSUSPENDED)
> |   case VIR_DOMAIN_PMSUSPENDED:
> |     ....
> | #endif
> | }
> 
> but that is still more maintainable.
> 
> * include/libvirt/libvirt.h.in (virDomainState): Expose #defines
> matching each enum value.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
> 
> This patch is an RFC because I want confirmation that it is worth
> doing.  Obviously, if it is desirable, there will be a LOT more
> addition of #define throughout the file, but as that is mostly
> busy-work, I want to get the idea approved first.
> 
>  include/libvirt/libvirt.h.in | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index c910b31..0baea53 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -116,13 +116,28 @@ typedef virDomain *virDomainPtr;
>   * A domain may be in different states at a given point in time
>   */
>  typedef enum {
> +#define VIR_DOMAIN_NOSTATE VIR_DOMAIN_NOSTATE
>      VIR_DOMAIN_NOSTATE = 0,     /* no state */
> +
> +#define VIR_DOMAIN_RUNNING VIR_DOMAIN_RUNNING
>      VIR_DOMAIN_RUNNING = 1,     /* the domain is running */
> +
> +#define VIR_DOMAIN_BLOCKED VIR_DOMAIN_BLOCKED
>      VIR_DOMAIN_BLOCKED = 2,     /* the domain is blocked on resource */
> +
> +#define VIR_DOMAIN_PAUSED VIR_DOMAIN_PAUSED
>      VIR_DOMAIN_PAUSED  = 3,     /* the domain is paused by user */
> +
> +#define VIR_DOMAIN_SHUTDOWN VIR_DOMAIN_SHUTDOWN
>      VIR_DOMAIN_SHUTDOWN= 4,     /* the domain is being shut down */
> +
> +#define VIR_DOMAIN_SHUTOFF VIR_DOMAIN_SHUTOFF
>      VIR_DOMAIN_SHUTOFF = 5,     /* the domain is shut off */
> +
> +#define VIR_DOMAIN_CRASHED VIR_DOMAIN_CRASHED
>      VIR_DOMAIN_CRASHED = 6,     /* the domain is crashed */
> +
> +#define VIR_DOMAIN_PMSUSPENDED VIR_DOMAIN_PMSUSPENDED
>      VIR_DOMAIN_PMSUSPENDED = 7, /* the domain is suspended by guest
>                                     power management */

This is pretty damn ugly IMHO.  I'd only support that if it was entirely
automatically generated as part of the libvirt.h.in -> libvirt.h
conversion.

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