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