11:58 p.m.
Since v1:
- push patch 1, 2 that had successful reviews
- new patch 1
- rewrite old patch 3 to remove all use of gnulib from examples/,
rather than being partitioned based on what uses config.h (in part
by reverting Jan's recent patch that added config.h everywhere)
Eric Blake (2):
examples: Drop event-test.c dependency on gnulib <verify.h>
examples: Avoid gnulib, have standalone examples
examples/Makefile.am | 5 ++---
examples/admin/client_close.c | 2 --
examples/admin/client_info.c | 3 +--
examples/admin/client_limits.c | 2 --
examples/admin/list_clients.c | 2 --
examples/admin/list_servers.c | 2 --
examples/admin/logging.c | 2 --
examples/admin/threadpool_params.c | 2 --
examples/dommigrate/dommigrate.c | 2 --
examples/domsuspend/suspend.c | 2 --
examples/domtop/domtop.c | 2 --
examples/hellolibvirt/hellolibvirt.c | 2 --
examples/object-events/event-test.c | 12 ++++++++----
examples/openauth/openauth.c | 2 --
examples/rename/rename.c | 2 --
15 files changed, 11 insertions(+), 33 deletions(-)
--
2.20.1
11:58 p.m.
New subject: [libvirt] [PATCH v2 1/2] examples: Drop event-test.c dependency on gnulib <verify.h>
Pulling in gnulib just for the <verify.h> header is rather
expensive, especially since that header does not require us
to link against gnulib. It's better to make the event-test
example be standalone by just open-coding a more limited form
of a verify() macro that depends on modern gcc (we have enough
CI coverage that even though the verify is now a no-op in
older setups, we will still notice if we fail to add an event
- as a quick test, I still provoked a compile failure on
Fedora 29 by deleting a line from domainEvents).
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
examples/object-events/event-test.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/examples/object-events/event-test.c b/examples/object-events/event-test.c
index 8499e0b38e..6694d3dffc 100644
--- a/examples/object-events/event-test.c
+++ b/examples/object-events/event-test.c
@@ -6,8 +6,6 @@
#include <signal.h>
#include <inttypes.h>
-#include <verify.h>
-
#define VIR_ENUM_SENTINELS
#include <libvirt/libvirt.h>
@@ -17,6 +15,14 @@
#define STREQ(a, b) (strcmp(a, b) == 0)
#define NULLSTR(s) ((s) ? (s) : "<null>")
+#if (4 < __GNUC__ + (6 <= __GNUC_MINOR__) \
+ && (201112L <= __STDC_VERSION__ || !defined __STRICT_ANSI__) \
+ && !defined __cplusplus)
+# define verify(cond) _Static_assert(cond, "verify (" #cond ")")
+#else
+# define verify(cond)
+#endif
+
#ifndef ATTRIBUTE_UNUSED
# define ATTRIBUTE_UNUSED __attribute__((__unused__))
#endif
--
2.20.1
11:58 p.m.
New subject: [libvirt] [PATCH v2 2/2] examples: Avoid gnulib, have standalone examples
Commit 0c6ad476 updated gnulib, which rearranged some of the
conditions in gnulib wrapper headers such that compilation
started failing on BSD systems when the normal system <unistd.h>
tried to include another system header but instead got a
gnulib wrapper header in an incomplete state; this is because
gnulib headers only work if <config.h> is included first.
Commit b6f78259 papered over the symptoms of that by including
<config.h> in all the examples. But this logic is backwards -
if our examples are truly meant to be stand-alone, they should
NOT depend on how libvirt was configured, and should NOT
depend on the gnulib fixes for system quirks. In particular,
if an example does not need to link against libgnulib.la,
then it also does not need to use -Ignulib in its compile
flags, and likewise does not need to include <config.h> since
none of the gnulib wrapper headers should be interfering.
So, revert (most of) b6f78259 (except for the bogus pre-patch
use of "config.h" in admin/logging.c: if config.h is included,
it should be via <> rather than "", and must be before any
system headers); then additionally nuke all mention of
<config.h>, -Ignulib, and -lgnulib, making all of the examples
truly standalone.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
v2: rewrite to nuke rather than partition use of gnulib, revert
Jan's patch that landed in meantime
---
examples/Makefile.am | 5 ++---
examples/admin/client_close.c | 2 --
examples/admin/client_info.c | 3 +--
examples/admin/client_limits.c | 2 --
examples/admin/list_clients.c | 2 --
examples/admin/list_servers.c | 2 --
examples/admin/logging.c | 2 --
examples/admin/threadpool_params.c | 2 --
examples/dommigrate/dommigrate.c | 2 --
examples/domsuspend/suspend.c | 2 --
examples/domtop/domtop.c | 2 --
examples/hellolibvirt/hellolibvirt.c | 2 --
examples/object-events/event-test.c | 2 --
examples/openauth/openauth.c | 2 --
examples/rename/rename.c | 2 --
15 files changed, 3 insertions(+), 31 deletions(-)
diff --git a/examples/Makefile.am b/examples/Makefile.am
index 61cf2af4a5..76907a1c8f 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -34,10 +34,9 @@ EXTRA_DIST = \
AM_CPPFLAGS = \
- -I$(top_builddir)/include -I$(top_srcdir)/include -I$(top_srcdir) \
- -I$(top_builddir)/gnulib/lib -I$(top_srcdir)/gnulib/lib
+ -I$(top_builddir)/include -I$(top_srcdir)/include -I$(top_srcdir)
LDADD = $(STATIC_BINARIES) $(WARN_CFLAGS) $(COVERAGE_LDFLAGS) \
- $(top_builddir)/src/libvirt.la $(top_builddir)/gnulib/lib/libgnu.la \
+ $(top_builddir)/src/libvirt.la \
$(top_builddir)/src/libvirt-admin.la
noinst_PROGRAMS=dominfo/info1 dommigrate/dommigrate domsuspend/suspend \
diff --git a/examples/admin/client_close.c b/examples/admin/client_close.c
index 9b7e1febb1..5c28ce7949 100644
--- a/examples/admin/client_close.c
+++ b/examples/admin/client_close.c
@@ -1,5 +1,3 @@
-#include <config.h>
-
#include <stdio.h>
#include <stdlib.h>
#include <libvirt/libvirt.h>
diff --git a/examples/admin/client_info.c b/examples/admin/client_info.c
index 550a24824a..2c46ccf514 100644
--- a/examples/admin/client_info.c
+++ b/examples/admin/client_info.c
@@ -1,5 +1,4 @@
-#include <config.h>
-
+#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
diff --git a/examples/admin/client_limits.c b/examples/admin/client_limits.c
index a87cb52c82..69b931682c 100644
--- a/examples/admin/client_limits.c
+++ b/examples/admin/client_limits.c
@@ -1,5 +1,3 @@
-#include <config.h>
-
#include <stdio.h>
#include <stdlib.h>
#include <libvirt/libvirt-admin.h>
diff --git a/examples/admin/list_clients.c b/examples/admin/list_clients.c
index 1d961a1ec9..15205a7bd1 100644
--- a/examples/admin/list_clients.c
+++ b/examples/admin/list_clients.c
@@ -1,5 +1,3 @@
-#include <config.h>
-
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
diff --git a/examples/admin/list_servers.c b/examples/admin/list_servers.c
index d48d87af6e..e53cd3b48f 100644
--- a/examples/admin/list_servers.c
+++ b/examples/admin/list_servers.c
@@ -1,5 +1,3 @@
-#include <config.h>
-
#include <stdio.h>
#include <stdlib.h>
#include <libvirt/libvirt-admin.h>
diff --git a/examples/admin/logging.c b/examples/admin/logging.c
index 10952afa03..730ae40d9d 100644
--- a/examples/admin/logging.c
+++ b/examples/admin/logging.c
@@ -1,5 +1,3 @@
-#include <config.h>
-
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
diff --git a/examples/admin/threadpool_params.c b/examples/admin/threadpool_params.c
index 9517dcb0f2..be833c2ea3 100644
--- a/examples/admin/threadpool_params.c
+++ b/examples/admin/threadpool_params.c
@@ -1,5 +1,3 @@
-#include <config.h>
-
#include <stdio.h>
#include <stdlib.h>
#include <libvirt/libvirt-admin.h>
diff --git a/examples/dommigrate/dommigrate.c b/examples/dommigrate/dommigrate.c
index 71a0af4058..1b6072d138 100644
--- a/examples/dommigrate/dommigrate.c
+++ b/examples/dommigrate/dommigrate.c
@@ -20,8 +20,6 @@
* <http://www.gnu.org/licenses/>;.
*/
-#include <config.h>
-
#include <stdio.h>
#include <stdlib.h>
#include <libvirt/libvirt.h>
diff --git a/examples/domsuspend/suspend.c b/examples/domsuspend/suspend.c
index ac816044ef..af61129ecc 100644
--- a/examples/domsuspend/suspend.c
+++ b/examples/domsuspend/suspend.c
@@ -19,8 +19,6 @@
* <http://www.gnu.org/licenses/>;.
*/
-#include <config.h>
-
#include <errno.h>
#include <getopt.h>
#include <libvirt/libvirt.h>
diff --git a/examples/domtop/domtop.c b/examples/domtop/domtop.c
index fb631781fd..4224e4c9ec 100644
--- a/examples/domtop/domtop.c
+++ b/examples/domtop/domtop.c
@@ -18,8 +18,6 @@
* <http://www.gnu.org/licenses/>;.
*/
-#include <config.h>
-
#include <errno.h>
#include <getopt.h>
#include <libvirt/libvirt.h>
diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellolibvirt.c
index 02c4401987..bfb12846e6 100644
--- a/examples/hellolibvirt/hellolibvirt.c
+++ b/examples/hellolibvirt/hellolibvirt.c
@@ -2,8 +2,6 @@
* hypervisor and gather a few bits of information about domains.
* Similar API's exist for storage pools, networks, and interfaces. */
-#include <config.h>
-
#include <stdio.h>
#include <stdlib.h>
#include <libvirt/libvirt.h>
diff --git a/examples/object-events/event-test.c b/examples/object-events/event-test.c
index 6694d3dffc..53d95b10d0 100644
--- a/examples/object-events/event-test.c
+++ b/examples/object-events/event-test.c
@@ -1,5 +1,3 @@
-#include <config.h>
-
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
diff --git a/examples/openauth/openauth.c b/examples/openauth/openauth.c
index eef46d5f52..efd21c374f 100644
--- a/examples/openauth/openauth.c
+++ b/examples/openauth/openauth.c
@@ -1,8 +1,6 @@
/* This is a copy of the hellolibvirt example demonstaring how to use
* virConnectOpenAuth with a custom auth callback */
-#include <config.h>
-
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
diff --git a/examples/rename/rename.c b/examples/rename/rename.c
index c64dd5baa3..85f18e9df3 100644
--- a/examples/rename/rename.c
+++ b/examples/rename/rename.c
@@ -16,8 +16,6 @@
* <http://www.gnu.org/licenses/>;.
*/
-#include <config.h>
-
#include <stdio.h>
#include <stdlib.h>
#include <libvirt/libvirt.h>
--
2.20.1
10:32 a.m.
New subject: [libvirt] [PATCH v2 2/2] examples: Avoid gnulib, have standalone examples
On 1/7/19 11:58 PM, Eric Blake wrote:
Commit 0c6ad476 updated gnulib, which rearranged some of the
conditions in gnulib wrapper headers such that compilation
started failing on BSD systems when the normal system <unistd.h>
tried to include another system header but instead got a
gnulib wrapper header in an incomplete state; this is because
gnulib headers only work if <config.h> is included first.
Commit b6f78259 papered over the symptoms of that by including
<config.h> in all the examples. But this logic is backwards -
if our examples are truly meant to be stand-alone, they should
NOT depend on how libvirt was configured, and should NOT
depend on the gnulib fixes for system quirks. In particular,
if an example does not need to link against libgnulib.la,
then it also does not need to use -Ignulib in its compile
flags, and likewise does not need to include <config.h> since
none of the gnulib wrapper headers should be interfering.
So, revert (most of) b6f78259 (except for the bogus pre-patch
use of "config.h" in admin/logging.c: if config.h is included,
it should be via <> rather than "", and must be before any
system headers); then additionally nuke all mention of
<config.h>, -Ignulib, and -lgnulib, making all of the examples
truly standalone.
Apparently it's now too standalone - Travis complains that mingw now fails:
domtop/domtop.c: In function 'print_cpu_usage':
domtop/domtop.c:229:15: warning: unknown conversion type character 'l'
in format [-Wformat=]
DEBUG("now_params=%llu then_params=%llu now=%llu then=%llu",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...
domtop/domtop.c: In function 'do_top':
domtop/domtop.c:268:22: error: storage size of 'action_stop' isn't known
struct sigaction action_stop;
^~~~~~~~~~~
so at least domtop.c was actually depending on gnulib fixing printf and
providing sigaction. Rewriting the printf uses to favor PRIu64 might
help, but I'm not sure how to work around missing sigaction. I'm trying
to reproduce the mingw cross-build locally, rather than iteratively
hammering through more Travis tests, and hope to post a fixup patch
later today. It may be that I go back to my earlier plan where some of
the examples link against gnulib (but in that case, it will be just the
tests that need it, rather than all the tests).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
3:22 a.m.
On 1/8/19 6:58 AM, Eric Blake wrote:
Since v1:
- push patch 1, 2 that had successful reviews
- new patch 1
- rewrite old patch 3 to remove all use of gnulib from examples/,
rather than being partitioned based on what uses config.h (in part
by reverting Jan's recent patch that added config.h everywhere)
Eric Blake (2):
examples: Drop event-test.c dependency on gnulib <verify.h>
examples: Avoid gnulib, have standalone examples
examples/Makefile.am | 5 ++---
examples/admin/client_close.c | 2 --
examples/admin/client_info.c | 3 +--
examples/admin/client_limits.c | 2 --
examples/admin/list_clients.c | 2 --
examples/admin/list_servers.c | 2 --
examples/admin/logging.c | 2 --
examples/admin/threadpool_params.c | 2 --
examples/dommigrate/dommigrate.c | 2 --
examples/domsuspend/suspend.c | 2 --
examples/domtop/domtop.c | 2 --
examples/hellolibvirt/hellolibvirt.c | 2 --
examples/object-events/event-test.c | 12 ++++++++----
examples/openauth/openauth.c | 2 --
examples/rename/rename.c | 2 --
15 files changed, 11 insertions(+), 33 deletions(-)
ACK to both.
Michal
5:07 a.m.
On Mon, Jan 07, 2019 at 11:58:40PM -0600, Eric Blake wrote:
Since v1:
- push patch 1, 2 that had successful reviews
- new patch 1
- rewrite old patch 3 to remove all use of gnulib from examples/,
rather than being partitioned based on what uses config.h (in part
by reverting Jan's recent patch that added config.h everywhere)
Eric Blake (2):
examples: Drop event-test.c dependency on gnulib <verify.h>
examples: Avoid gnulib, have standalone examples
examples/Makefile.am | 5 ++---
examples/admin/client_close.c | 2 --
examples/admin/client_info.c | 3 +--
examples/admin/client_limits.c | 2 --
examples/admin/list_clients.c | 2 --
examples/admin/list_servers.c | 2 --
examples/admin/logging.c | 2 --
examples/admin/threadpool_params.c | 2 --
examples/dommigrate/dommigrate.c | 2 --
examples/domsuspend/suspend.c | 2 --
examples/domtop/domtop.c | 2 --
examples/hellolibvirt/hellolibvirt.c | 2 --
examples/object-events/event-test.c | 12 ++++++++----
examples/openauth/openauth.c | 2 --
examples/rename/rename.c | 2 --
15 files changed, 11 insertions(+), 33 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
2147
days inactive
2147
days old
5 comments
3 participants
participants (3)
-
Eric Blake -
Ján Tomko -
Michal Privoznik