Re: [Xen-devel] [xen-devel] [PATCH] libxl: fix compile error of libvirt

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [Xen-devel] [xen-devel] [PATCH] libxl: fix compile 
error of libvirt"):
> We should arrange for xl*.c to not have __XEN_TOOLS__ defined though.
> 
> -D__XEN_TOOLS__ is part of the base CFLAGS defined in tools/Rules.mk but
> perhaps we could add -U__XEN_TOOLS__ to the xl specific cflags?

That would be a possibility but if we define it in libxl.h that will
fix the problem, if we are happy for libxl callers to be using
hypervisor public headers.

> We can stop xl by just not doing it (TM), for other callers of libxl --
> well, we say you shouldn't need it for toolstack operations and that
> libxl should provide all the functionality but if they _really_ want to
> ignore that...
> 
> Also a toolstack may have functionality which is not considered
> "toolstack functionality" per the remit of libxl and for which they wish
> to use xenctrl directly. e.g. perhaps they communicate with a guest
> agent using their own shared ring protocol for which they require the
> ability to map domain memory.

How about the patch below.  This would be an alternative to your
version which buries all the hypervisor public headers.

Ian.

From: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Subject: [PATCH] libxl: Forbid <xenctrl.h>, allow Xen public headers

Rationalise and enforce the header-inclusion policy for libxl.

libxl applications are allowed to use symbols (mostly #defines) from
the Xen public headers, where these are useful (for example, where
they are the numeric arguments to libxl calls).  This avoids us having
to veneer the whole of the Xen public API.  For this to work without
special measures on the part of the application, libxl.h should define
__XEN_TOOLS__.

However libxl applications are not normally expected to want to use
libxc directly, so take steps to prevent them including <xenctrl.h>
unless they declare (as libxl itself does) that doing so is OK by
defining LIBXL_ALLOW_XENCTRL.  (We have to add a hook at the end of
xenctrl.h so that we can spot the case where xenctrl.h is included
second.)

Make xc.c comply with the policy: remove the redundant inclusion of
xenctrl.h.

This patch should make life easier for out-of-tree libxl callers and
hopefully prevent future mistakes relating to api visibility.

Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

---
 tools/libxc/xenctrl.h        |    4 ++++
 tools/libxl/libxl.h          |   10 ++++++++++
 tools/libxl/libxl_internal.h |    2 ++
 tools/libxl/xl.c             |    1 -
 4 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index 73d24e5..8441b62 100644
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -2047,4 +2047,8 @@ int xc_compression_uncompress_page(xc_interface *xch, 
char *compbuf,
                                   unsigned long compbuf_size,
                                   unsigned long *compbuf_pos, char *dest);
 
+#ifdef XENCTRL_DO_INCLUDE
+#include XENCTRL_DO_INCLUDE
+#endif
+
 #endif /* XENCTRL_H */
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 1bffa03..86a308d 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -124,9 +124,19 @@
  * Therefore public functions which initialize a libxl__gc MUST call
  * libxl__free_all() before returning.
  */
+
+#if !defined(LIBXL_ALLOW_XENCTRL)
+#ifdef XENCTRL_H
+#error applications which use libxl should not normally use xenctrl.h too
+#endif
+#define XENCTRL_DO_INCLUDE <libxl.h> /* spot if xenctrl.h came second */
+#endif /* !defined(LIBXL_ALLOW_XENCTRL) */
+
 #ifndef LIBXL_H
 #define LIBXL_H
 
+#define __XEN_TOOLS__ 1
+
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdarg.h>
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 46e131a..478de48 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -17,6 +17,8 @@
 #ifndef LIBXL_INTERNAL_H
 #define LIBXL_INTERNAL_H
 
+#define LIBXL_ALLOW_XENCTRL
+
 #include "libxl_osdeps.h" /* must come before any other headers */
 
 #include <assert.h>
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index df9b1e7..2b14814 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -26,7 +26,6 @@
 #include <fcntl.h>
 #include <ctype.h>
 #include <inttypes.h>
-#include <xenctrl.h>
 
 #include "libxl.h"
 #include "libxl_utils.h"
-- 
tg: (fa6fc38..) t/xen/xl.xenctrl.forbid (depends on: t/xen/gitignore)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel