3:06 p.m.
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?
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(a)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 */
--
1.9.3
4:16 a.m.
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(a)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 :|
10:49 a.m.
On 10/07/2014 03:16 AM, Daniel P. Berrange wrote:
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.
That ship has already sailed. Looking at just RHEL 7.0, I see the
following bits have been backported:
VIR_MIGRAGE_PARAM_LISTEN_ADDRESS (already a #define)
VIR_STORAGE_VOL_NETDIR
VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER
VIR_DOMAIN_EVENT_DISK_DROP_MISSING_ON_START
or looking at RHEL 6.5:
VIR_DOMAIN_RUNNING_CRASHED
VIR_DOMAIN_PAUSED_SNAPSHOT
VIR_DOMAIN_PAUSED_CRASHED
VIR_DOMAIN_CRASHED_PANICKED
VIR_DOMAIN_PMSUSPENDED_DISK_UNKNOWN
VIR_MIGRATE_ABORT_ON_ERROR
VIR_DOMAIN_XML_MIGRATABLE
VIR_DOMAIN_EVENT_CRASHED
VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR
VIR_DOMAIN_EVENT_PMSUSPENDED_DISK
VIR_DOMAIN_EVENT_CRASHED_PANICKED
VIR_DOMAIN_SNAPSHOT_CREATE_LIVE
VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE
VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE
VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY
VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL
VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL
VIR_DOMAIN_BLOCK_JOB_READY
VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK
VIR_DOMAIN_MEMORY_SHARED_MERGE_ACROSS_NODES (already a #define)
In other words, it's a VERY common action for downstreams to
conditionally backport new feature bits of existing APIs. Downstream
language bindings needs to reflect these additional bits. In the past,
when libvirt-python was part of libvirt.git, the python bindings got
this for free. But the bugzilla I mentioned above is a case where RHEL
7.1 is considering the backport of a new VIR_DOMAIN_EVENT_ID_* bit where
gating on version alone is insufficient for writing correct libvirt
python bindings; since the python bindings live in a different
repository and need SOME witness of whether the event has been
backported, since it is easy to backport new events without breaking ABI.
Writing the upstream libvirt-python bindings to use witness variables
instead of version numbers as the gate for any use of a particular
feature (especially VIR_DOMAIN_EVENT_ID_*) makes it more robust to build
out-of-the-box with a downstream libvirt that has backported feature
bits. On the other hand, the python bindings have already solved most
of the backport issue by scraping the XML document of all exported bits
as generated by the downstream libvirt, so new flags bits automatically
get exposed by the code generators. I guess it may require auditing the
libvirt-python bindings to see all use of LIBVIR_CHECK_VERSION to see
which uses are NOT tied to an ABI change, and maybe special-case that
hand-written C code to instead play off of something that the generator
can produce automatically based off of what it scrapes from the XML. If
THAT works, then I could patch just libvirt-python without worrying
about uglifying libvirt.h.
> 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,
> +
> +#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.
That's a fair point. If my experiments with trying to solve the issue
from the libvirt-python side don't pan out, then I'll look at what it
would take to automate things, so that it is not a maintainer burden.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3680
days inactive
3681
days old
2 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Eric Blake