Xen devel - Feb 2012 - [PATCH] libxl: fix compile error of libvirt

a, libxl_event.h is included in libxl.h. So, the former one also need to be
installed.
b, define __XEN_TOOLS__ in libxl.h: 
the head file "xen/sysctl.h" need check this macro.  
It is the same way used by the xen libxc public headers(tools/libxc/xenctrl.h
and tools/libxc/xenctrlosdep.h).

Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com> 

diff -r 87218bd367be tools/libxl/Makefile 
--- a/tools/libxl/MakefileFri Feb 17 12:24:38 2012 +0000 
+++ b/tools/libxl/MakefileMon Feb 20 15:36:09 2012 +0800 
@@ -163,7 +163,7 @@ 
 ln -sf libxlutil.so.$(XLUMAJOR).$(XLUMINOR)
$(DESTDIR)$(LIBDIR)/libxlutil.so.$(XLUMAJOR)
 ln -sf libxlutil.so.$(XLUMAJOR) $(DESTDIR)$(LIBDIR)/libxlutil.so 
 $(INSTALL_DATA) libxlutil.a $(DESTDIR)$(LIBDIR) 
-$(INSTALL_DATA) libxl.h libxl_json.h _libxl_types.h _libxl_types_json.h
_libxl_list.h libxl_utils.h libxl_uuid.h $(DESTDIR)$(INCLUDEDIR)
+$(INSTALL_DATA) libxl.h libxl_json.h _libxl_types.h _libxl_types_json.h
_libxl_list.h libxl_utils.h libxl_uuid.h libxl_event.h $(DESTDIR)$(INCLUDEDIR)
 $(INSTALL_DATA) bash-completion $(DESTDIR)$(BASH_COMPLETION_DIR)/xl.sh 
  
 .PHONY: clean 

diff -r 87218bd367be tools/libxl/libxl.h 
--- a/tools/libxl/libxl.hFri Feb 17 12:24:38 2012 +0000 
+++ b/tools/libxl/libxl.hMon Feb 20 15:36:09 2012 +0800 
@@ -127,6 +127,11 @@ 
 #ifndef LIBXL_H 
 #define LIBXL_H 
  
+/* Tell the Xen public headers we are a user-space tools build. */ 
+#ifndef __XEN_TOOLS__ 
+#define __XEN_TOOLS__ 1 
+#endif  
+ 
 #include <stdbool.h> 
 #include <stdint.h> 
 #include <stdarg.h> 


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

a, libxl_event.h is included in libxl.h. So, the former one also need to be
installed.
b, define __XEN_TOOLS__ in libxl.h: 
the head file "xen/sysctl.h" need check this macro.  
It is the same way used by the xen libxc public headers(tools/libxc/xenctrl.h
and tools/libxc/xenctrlosdep.h).


Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com> 


diff -r 87218bd367be tools/libxl/Makefile 
--- a/tools/libxl/MakefileFri Feb 17 12:24:38 2012 +0000 
+++ b/tools/libxl/MakefileMon Feb 20 15:36:09 2012 +0800 
@@ -163,7 +163,7 @@ 
 ln -sf libxlutil.so.$(XLUMAJOR).$(XLUMINOR)
$(DESTDIR)$(LIBDIR)/libxlutil.so.$(XLUMAJOR)
 ln -sf libxlutil.so.$(XLUMAJOR) $(DESTDIR)$(LIBDIR)/libxlutil.so 
 $(INSTALL_DATA) libxlutil.a $(DESTDIR)$(LIBDIR) 
-$(INSTALL_DATA) libxl.h libxl_json.h _libxl_types.h _libxl_types_json.h
_libxl_list.h libxl_utils.h libxl_uuid.h $(DESTDIR)$(INCLUDEDIR)
+$(INSTALL_DATA) libxl.h libxl_json.h _libxl_types.h _libxl_types_json.h
_libxl_list.h libxl_utils.h libxl_uuid.h libxl_event.h $(DESTDIR)$(INCLUDEDIR)
 $(INSTALL_DATA) bash-completion $(DESTDIR)$(BASH_COMPLETION_DIR)/xl.sh 
  
 .PHONY: clean 


diff -r 87218bd367be tools/libxl/libxl.h 
--- a/tools/libxl/libxl.hFri Feb 17 12:24:38 2012 +0000 
+++ b/tools/libxl/libxl.hMon Feb 20 15:36:09 2012 +0800 
@@ -127,6 +127,11 @@ 
 #ifndef LIBXL_H 
 #define LIBXL_H 
  
+/* Tell the Xen public headers we are a user-space tools build. */ 
+#ifndef __XEN_TOOLS__ 
+#define __XEN_TOOLS__ 1 
+#endif  
+ 
 #include <stdbool.h> 
 #include <stdint.h> 
 #include <stdarg.h> 


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

Bamvor Jian Zhang writes ("[Xen-devel] [xen-devel] [PATCH] libxl: fix
compile error of libvirt"):
> 
> a, libxl_event.h is included in libxl.h. So, the former one also need to be
> installed.
Well spotted.  However, I''m afraid your mail has been very badly
mangled by whatever program you used to send it.  For this patch I
reconstructed the change by hand and have committed it.
> b, define __XEN_TOOLS__ in libxl.h:
> the head file "xen/sysctl.h" need check this macro.
I don''t think this is correct.
> It is the same way used by the xen libxc public
headers(tools/libxc/xenctrl.h
> and tools/libxc/xenctrlosdep.h).
Users of libxl should not be using libxc directly and therefore should
not be including xenctrl.h.

Note that the API for libxl has changed in xen-unstable.hg compared to
4.1, and further changes are forthcoming.  So there will have to be
changes in libvirt.

Thanks,
Ian.

Ian Jackson wrote: 
> Bamvor Jian Zhang writes ("[Xen-devel] [xen-devel] [PATCH] libxl: fix
compile
> error of libvirt"): 
> >  
> > a, libxl_event.h is included in libxl.h. So, the former one also need
to be
> > installed. 
>  
> Well spotted.  However, I''m afraid your mail has been very badly 
> mangled by whatever program you used to send it.  For this patch I 
> reconstructed the change by hand and have committed it. 
>  
thanks. I will be careful in future.
> > b, define __XEN_TOOLS__ in libxl.h: 
> > the head file "xen/sysctl.h" need check this macro. 
>  
> I don''t think this is correct. 
>  
> > It is the same way used by the xen libxc public  
> headers(tools/libxc/xenctrl.h 
> > and tools/libxc/xenctrlosdep.h). 
>  
> Users of libxl should not be using libxc directly and therefore should 
> not be including xenctrl.h. 
>  
> Note that the API for libxl has changed in xen-unstable.hg compared to 
> 4.1, and further changes are forthcoming.  So there will have to be 
> changes in libvirt. 
>  
but after your commit "23174:751c6dcec0d4"(remove xenctrl.h from
libxl.h), the aplication(like libvirt) compile fail. How do i deal with it?
it seems that add __XEN_TOOLS_ to libvirt code is not good.
> Thanks, 
> Ian. 
>  
Thanks
bamvor

Bamvor Jian Zhang writes ("Re: [Xen-devel] [xen-devel] [PATCH] libxl: fix
compile error of libvirt"):
> Ian Jackson wrote: 
> > Users of libxl should not be using libxc directly and therefore should
> > not be including xenctrl.h. 
...
> but after your commit "23174:751c6dcec0d4"(remove xenctrl.h from
libxl.h), the aplication(like libvirt) compile fail. How do i deal with it?
> it seems that add __XEN_TOOLS_ to libvirt code is not good. 
Can you tell us the error message you get ?  I think the problem is
probably that libvirt is trying to use libxc directly.

Now there is an existing libxc/libxenstore-based libvirt driver for
Xen but that should be in a separate file.  The libxl-based driver
should not use libxc.

Ian.

On Wed, 2012-02-22 at 11:52 +0000, Ian Jackson wrote:
> Bamvor Jian Zhang writes ("Re: [Xen-devel] [xen-devel] [PATCH] libxl:
fix compile error of libvirt"):
> > Ian Jackson wrote: 
> > > Users of libxl should not be using libxc directly and therefore
should
> > > not be including xenctrl.h. 
> ...
> > but after your commit "23174:751c6dcec0d4"(remove xenctrl.h
from libxl.h), the aplication(like libvirt) compile fail. How do i deal with it?
> > it seems that add __XEN_TOOLS_ to libvirt code is not good. 
> 
> Can you tell us the error message you get ?  I think the problem is
> probably that libvirt is trying to use libxc directly.
Is the problem here libxc or the Xen public headers?

I thought we''d decided the latter was OK in users of libxl. e.g. xl has
to use the SHUTDOWN_{poweroff,reboot,suspend} constants from
xen/include/public/sched.h (and indeed libxl.h includes this header).

libxl.h also includes Xen''s sysctl.h -- seems to be for
XEN_SYSCTL_PHYSCAP_*. Really this case would be better exposing as a
series of bools which are initialised from the hypercall provided flags
mask by libxl (like we do for the various dominfo flags)

Personally I''d be happy to have libxl always define its own constants
and remove this need for libxl users to use the Xen headers.

If we are going to do that it should go onto the 4.2 TODO list.

Ian.

Ian Jackson wrote:
> Bamvor Jian Zhang writes ("Re: [Xen-devel] [xen-devel] [PATCH] libxl:
fix compile error of libvirt"):
>   
>> Ian Jackson wrote: 
>>     
>>> Users of libxl should not be using libxc directly and therefore
should
>>> not be including xenctrl.h. 
>>>       
> ...
>   
>> but after your commit "23174:751c6dcec0d4"(remove xenctrl.h
from libxl.h), the aplication(like libvirt) compile fail. How do i deal with it?
>> it seems that add __XEN_TOOLS_ to libvirt code is not good. 
>>     
>
> Can you tell us the error message you get ?  I think the problem is
> probably that libvirt is trying to use libxc directly.
>   
The libvirt libxl driver doesn''t use libxc directly. AFAICT, the
problem
is that libxl.h includes <xen/sysctl.h>, which has this

#if !defined(__XEN__) && !defined(__XEN_TOOLS__)
#error "sysctl operations are intended for use by node control tools
only"
#endif

Without the defines, Bamvor is hitting the #error directive.

Regards,
Jim

Ian Jackson wrote:
> Note that the API for libxl has changed in xen-unstable.hg compared to
> 4.1, and further changes are forthcoming.  So there will have to be
> changes in libvirt.
>   
I suppose libvirt is the only out-of-tree libxl app feeling this pain :-(.

Regards,
Jim

Ian Jackson  wrote: 
>Bamvor Jian Zhang writes ("Re: [Xen-devel] [xen-devel] [PATCH] libxl:
fix compile error of libvirt"):
>> Ian Jackson wrote: 
>> > Users of libxl should not be using libxc directly and therefore
should
>> > not be including xenctrl.h. 
>...
>> but after your commit "23174:751c6dcec0d4"(remove xenctrl.h
from libxl.h), the aplication(like libvirt) compile fail. How do i deal with it?
>> it seems that add __XEN_TOOLS_ to libvirt code is not good. 
>
>Can you tell us the error message you get ?  I think the problem is
>probably that libvirt is trying to use libxc directly.
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -I../gnulib/lib
-I../gnulib/lib -I../include -I../include -I../src/util -DIN_LIBVIRT
-I../src/conf -I../src/xenxs -I/usr/include/libxml2 -Wall -W -Wformat-y2k
-Wformat-security -Winit-self -Wmissing-include-dirs -Wunused -Wunknown-pragmas
-Wstrict-aliasing -Wshadow -Wpointer-arith -Wbad-function-cast -Wcast-align
-Wwrite-strings -Wlogical-op -Waggregate-return -Wstrict-prototypes
-Wold-style-definition -Wmissing-prototypes -Wmissing-declarations
-Wmissing-noreturn -Wmissing-format-attribute -Wredundant-decls -Wnested-externs
-Winline -Winvalid-pch -Wvolatile-register-var -Wdisabled-optimization
-Wattributes -Wcoverage-mismatch -Wmultichar -Wdeprecated-declarations
-Wdiv-by-zero -Wendif-labels -Wextra -Wformat-contains-nul -Wformat-
 extra-args -Wformat-zero-length -Wformat=2 -Wmultichar -Wnormalized=nfc
-Woverflow -Wpointer-to-int-cast -Wpragmas -Wno-missing-field-initializers
-Wno-sign-compare -Wno-format-nonliteral -fstack-protector-all
--param=ssp-buffer-size=4 -fexceptions -fasynchronous-unwind-tables
-fdiagnostics-show-option -funit-at-a-time -fipa-pure-const -Werror -g -O2 -MT
libvirt_driver_libxl_la-libxl_conf.lo -MD -MP -MF
.deps/libvirt_driver_libxl_la-libxl_conf.Tpo -c libxl/libxl_conf.c  -fPIC -DPIC
-o .libs/libvirt_driver_libxl_la-libxl_conf.o
In file included from /usr/include/libxl.h:140,
                 from libxl/libxl_conf.c:29:
/usr/include/xen/sysctl.h:31:2: error: #error "sysctl operations are
intended for use by node control tools only"
In file included from /usr/include/xen/sysctl.h:35,
                 from /usr/include/libxl.h:140,
                 from libxl/libxl_conf.c:29:
/usr/include/xen/domctl.h:32:2: error: #error "domctl operations are
intended for use by node control tools only"
In file included from /usr/include/xen/sysctl.h:35,
                 from /usr/include/libxl.h:140,
                 from libxl/libxl_conf.c:29:

>Now there is an existing libxc/libxenstore-based libvirt driver for
>Xen but that should be in a separate file.  The libxl-based driver
>should not use libxc.
>
>Ian.

On Wed, 2012-02-22 at 21:40 +0000, Jim Fehlig wrote:
> Ian Jackson wrote:
> > Bamvor Jian Zhang writes ("Re: [Xen-devel] [xen-devel] [PATCH]
libxl: fix compile error of libvirt"):
> >   
> >> Ian Jackson wrote: 
> >>     
> >>> Users of libxl should not be using libxc directly and
therefore should
> >>> not be including xenctrl.h. 
> >>>       
> > ...
> >   
> >> but after your commit "23174:751c6dcec0d4"(remove
xenctrl.h from libxl.h), the aplication(like libvirt) compile fail. How do i
deal with it?
> >> it seems that add __XEN_TOOLS_ to libvirt code is not good. 
> >>     
> >
> > Can you tell us the error message you get ?  I think the problem is
> > probably that libvirt is trying to use libxc directly.
> >   
> 
> The libvirt libxl driver doesn''t use libxc directly. AFAICT, the
problem
> is that libxl.h includes <xen/sysctl.h>, which has this
> 
> #if !defined(__XEN__) && !defined(__XEN_TOOLS__)
> #error "sysctl operations are intended for use by node control tools
only"
> #endif
> 
> Without the defines, Bamvor is hitting the #error directive.
I thought we''d discussed and resolved this ages ago but I guess we only
discussed it...

How about the following:

8<-------------------------------------------------

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329986233 0
# Node ID d256d6b42ee77cdff0356c37d6fcaf6203a21ba6
# Parent  aa82c27ea0a3cdfb5e58244918d2046cf4e314a9
libxl: remove sysctl.h from public interface

Using sysctl.h is restricted to "node control tools only" and requires
magic
defines. Therefore make its use internal to libxl.

Also removes an indirect include of domctl.h which has the same restrction.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r aa82c27ea0a3 -r d256d6b42ee7 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Wed Feb 22 14:43:57 2012 +0000
+++ b/tools/libxl/libxl.c	Thu Feb 23 08:37:13 2012 +0000
@@ -491,7 +491,7 @@ libxl_cpupoolinfo * libxl_list_cpupool(l
         }
         ptr = tmp;
         ptr[i].poolid = info->cpupool_id;
-        ptr[i].sched_id = info->sched_id;
+        ptr[i].sched = info->sched_id;
         ptr[i].n_dom = info->n_dom;
         if (libxl_cpumap_alloc(ctx, &ptr[i].cpumap)) {
             xc_cpupool_infofree(ctx->xch, info);
@@ -2750,7 +2750,10 @@ int libxl_get_physinfo(libxl_ctx *ctx, l
     physinfo->sharing_used_frames = xc_sharing_used_frames(ctx->xch);
     physinfo->nr_nodes = xcphysinfo.nr_nodes;
     memcpy(physinfo->hw_cap,xcphysinfo.hw_cap, sizeof(physinfo->hw_cap));
-    physinfo->phys_cap = xcphysinfo.capabilities;
+
+    physinfo->cap_hvm = !!(xcphysinfo.capabilities &
XEN_SYSCTL_PHYSCAP_hvm);
+    physinfo->cap_hvm_directio +        !!(xcphysinfo.capabilities &
XEN_SYSCTL_PHYSCAP_hvm_directio);
 
     return 0;
 }
@@ -2965,14 +2968,11 @@ out:
     return rc;
 }
 
-/*
- * returns one of the XEN_SCHEDULER_* constants from public/domctl.h
- */
-int libxl_get_sched_id(libxl_ctx *ctx)
+libxl_scheduler libxl_get_scheduler(libxl_ctx *ctx)
 {
-    int sched, ret;
-
-    if ((ret = xc_sched_id(ctx->xch, &sched)) != 0) {
+    libxl_scheduler sched, ret;
+
+    if ((ret = xc_sched_id(ctx->xch, (int *)&sched)) != 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info
list");
         return ERROR_FAIL;
     }
@@ -3445,7 +3445,8 @@ int libxl_get_freecpus(libxl_ctx *ctx, l
     return 0;
 }
 
-int libxl_cpupool_create(libxl_ctx *ctx, const char *name, int schedid,
+int libxl_cpupool_create(libxl_ctx *ctx, const char *name,
+                         libxl_scheduler sched,
                          libxl_cpumap cpumap, libxl_uuid *uuid,
                          uint32_t *poolid)
 {
@@ -3461,7 +3462,7 @@ int libxl_cpupool_create(libxl_ctx *ctx,
         return ERROR_NOMEM;
     }
 
-    rc = xc_cpupool_create(ctx->xch, poolid, schedid);
+    rc = xc_cpupool_create(ctx->xch, poolid, sched);
     if (rc) {
         LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
            "Could not create cpupool");
diff -r aa82c27ea0a3 -r d256d6b42ee7 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Wed Feb 22 14:43:57 2012 +0000
+++ b/tools/libxl/libxl.h	Thu Feb 23 08:37:13 2012 +0000
@@ -138,7 +138,6 @@
 #include <xentoollog.h>
 
 #include <xen/sched.h>
-#include <xen/sysctl.h>
 
 #include <libxl_uuid.h>
 #include <_libxl_list.h>
@@ -584,7 +583,7 @@ int libxl_set_vcpuaffinity_all(libxl_ctx
                                unsigned int max_vcpus, libxl_cpumap *cpumap);
 int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_cpumap *cpumap);
 
-int libxl_get_sched_id(libxl_ctx *ctx);
+libxl_scheduler libxl_get_scheduler(libxl_ctx *ctx);
 
 
 int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
@@ -627,7 +626,8 @@ int libxl_tmem_shared_auth(libxl_ctx *ct
 int libxl_tmem_freeable(libxl_ctx *ctx);
 
 int libxl_get_freecpus(libxl_ctx *ctx, libxl_cpumap *cpumap);
-int libxl_cpupool_create(libxl_ctx *ctx, const char *name, int schedid,
+int libxl_cpupool_create(libxl_ctx *ctx, const char *name,
+                         libxl_scheduler sched,
                          libxl_cpumap cpumap, libxl_uuid *uuid,
                          uint32_t *poolid);
 int libxl_cpupool_destroy(libxl_ctx *ctx, uint32_t poolid);
diff -r aa82c27ea0a3 -r d256d6b42ee7 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Wed Feb 22 14:43:57 2012 +0000
+++ b/tools/libxl/libxl_types.idl	Thu Feb 23 08:37:13 2012 +0000
@@ -99,6 +99,14 @@ libxl_timer_mode = Enumeration("timer_mo
     (3, "one_missed_tick_pending"),
     ])
 
+# Consistent with values defined in domctl.h
+libxl_scheduler = Enumeration("scheduler", [
+    (4, "sedf"),
+    (5, "credit"),
+    (6, "credit2"),
+    (7, "arinc653"),
+    ])
+
 #
 # Complex libxl types
 #
@@ -158,7 +166,7 @@ libxl_dominfo = Struct("dominfo",[
 
 libxl_cpupoolinfo = Struct("cpupoolinfo", [
     ("poolid",      uint32),
-    ("sched_id",    uint32),
+    ("sched",       libxl_scheduler),
     ("n_dom",       uint32),
     ("cpumap",      libxl_cpumap)
     ])
@@ -381,7 +389,9 @@ libxl_physinfo = Struct("physinfo", [
 
     ("nr_nodes", uint32),
     ("hw_cap", libxl_hwcap),
-    ("phys_cap", uint32),
+
+    ("cap_hvm", bool),
+    ("cap_hvm_directio", bool),
     ], dispose_fn=None, dir=DIR_OUT)
 
 libxl_cputopology = Struct("cputopology", [
diff -r aa82c27ea0a3 -r d256d6b42ee7 tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c	Wed Feb 22 14:43:57 2012 +0000
+++ b/tools/libxl/libxl_utils.c	Thu Feb 23 08:37:13 2012 +0000
@@ -19,18 +19,6 @@
 
 #include "libxl_internal.h"
 
-struct schedid_name {
-    char *name;
-    int id;
-};
-
-static struct schedid_name schedid_name[] = {
-    { "credit", XEN_SCHEDULER_CREDIT },
-    { "sedf", XEN_SCHEDULER_SEDF },
-    { "credit2", XEN_SCHEDULER_CREDIT2 },
-    { NULL, -1 }
-};
-
 const char *libxl_basename(const char *name)
 {
     const char *filename;
@@ -151,28 +139,6 @@ int libxl_name_to_cpupoolid(libxl_ctx *c
     return ret;
 }
 
-int libxl_name_to_schedid(libxl_ctx *ctx, const char *name)
-{
-    int i;
-
-    for (i = 0; schedid_name[i].name != NULL; i++)
-        if (strcmp(name, schedid_name[i].name) == 0)
-            return schedid_name[i].id;
-
-    return ERROR_INVAL;
-}
-
-char *libxl_schedid_to_name(libxl_ctx *ctx, int schedid)
-{
-    int i;
-
-    for (i = 0; schedid_name[i].name != NULL; i++)
-        if (schedid_name[i].id == schedid)
-            return schedid_name[i].name;
-
-    return "unknown";
-}
-
 int libxl_get_stubdom_id(libxl_ctx *ctx, int guest_domid)
 {
     GC_INIT(ctx);
diff -r aa82c27ea0a3 -r d256d6b42ee7 tools/libxl/libxl_utils.h
--- a/tools/libxl/libxl_utils.h	Wed Feb 22 14:43:57 2012 +0000
+++ b/tools/libxl/libxl_utils.h	Thu Feb 23 08:37:13 2012 +0000
@@ -24,8 +24,6 @@ int libxl_name_to_domid(libxl_ctx *ctx, 
 char *libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid);
 int libxl_name_to_cpupoolid(libxl_ctx *ctx, const char *name, uint32_t
*poolid);
 char *libxl_cpupoolid_to_name(libxl_ctx *ctx, uint32_t poolid);
-int libxl_name_to_schedid(libxl_ctx *ctx, const char *name);
-char *libxl_schedid_to_name(libxl_ctx *ctx, int schedid);
 int libxl_get_stubdom_id(libxl_ctx *ctx, int guest_domid);
 int libxl_is_stubdom(libxl_ctx *ctx, uint32_t domid, uint32_t *target_domid);
 int libxl_create_logfile(libxl_ctx *ctx, char *name, char **full_name);
diff -r aa82c27ea0a3 -r d256d6b42ee7 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Wed Feb 22 14:43:57 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Thu Feb 23 08:37:13 2012 +0000
@@ -3688,15 +3688,15 @@ int main_vcpuset(int argc, char **argv)
 static void output_xeninfo(void)
 {
     const libxl_version_info *info;
-    int sched_id;
+    libxl_scheduler sched;
 
     if (!(info = libxl_get_version_info(ctx))) {
         fprintf(stderr, "libxl_get_version_info failed.\n");
         return;
     }
 
-    if ((sched_id = libxl_get_sched_id(ctx)) < 0) {
-        fprintf(stderr, "get_sched_id sysctl failed.\n");
+    if ((sched = libxl_get_scheduler(ctx)) < 0) {
+        fprintf(stderr, "get_scheduler sysctl failed.\n");
         return;
     }
 
@@ -3704,7 +3704,7 @@ static void output_xeninfo(void)
     printf("xen_minor              : %d\n",
info->xen_version_minor);
     printf("xen_extra              : %s\n",
info->xen_version_extra);
     printf("xen_caps               : %s\n", info->capabilities);
-    printf("xen_scheduler          : %s\n",
libxl_schedid_to_name(ctx, sched_id));
+    printf("xen_scheduler          : %s\n",
libxl_scheduler_to_string(sched));
     printf("xen_pagesize           : %u\n", info->pagesize);
     printf("platform_params        :
virt_start=0x%"PRIx64"\n", info->virt_start);
     printf("xen_changeset          : %s\n", info->changeset);
@@ -3752,9 +3752,9 @@ static void output_physinfo(void)
     for (i = 0; i < 8; i++)
         printf("%08x%c", info.hw_cap[i], i < 7 ?
'':'' : ''\n'');
     printf("virt_caps              :");
-    if (info.phys_cap & XEN_SYSCTL_PHYSCAP_hvm)
+    if (info.cap_hvm)
         printf(" hvm");
-    if (info.phys_cap & XEN_SYSCTL_PHYSCAP_hvm_directio)
+    if (info.cap_hvm_directio)
         printf(" hvm_directio");
     printf("\n");
     vinfo = libxl_get_version_info(ctx);
@@ -4060,7 +4060,7 @@ static int sched_sedf_domain_output(
 }
 
 static int sched_domain_output(
-    uint32_t sched, int (*output)(int), const char *cpupool)
+    libxl_scheduler sched, int (*output)(int), const char *cpupool)
 {
     libxl_dominfo *info;
     libxl_cpupoolinfo *poolinfo = NULL;
@@ -4089,7 +4089,7 @@ static int sched_domain_output(
     }
 
     for (p = 0; !rc && (p < n_pools); p++) {
-        if ((poolinfo[p].sched_id != sched) ||
+        if ((poolinfo[p].sched != sched) ||
             (cpupool && (poolid != poolinfo[p].poolid)))
             continue;
 
@@ -4170,7 +4170,7 @@ int main_sched_credit(int argc, char **a
     }
 
     if (!dom) { /* list all domain''s credit scheduler info */
-        return -sched_domain_output(XEN_SCHEDULER_CREDIT,
+        return -sched_domain_output(LIBXL_SCHEDULER_CREDIT,
                                     sched_credit_domain_output, cpupool);
     } else {
         find_domain(dom);
@@ -4246,7 +4246,7 @@ int main_sched_credit2(int argc, char **
     }
 
     if (!dom) { /* list all domain''s credit scheduler info */
-        return -sched_domain_output(XEN_SCHEDULER_CREDIT2,
+        return -sched_domain_output(LIBXL_SCHEDULER_CREDIT2,
                                     sched_credit2_domain_output, cpupool);
     } else {
         find_domain(dom);
@@ -4348,7 +4348,7 @@ int main_sched_sedf(int argc, char **arg
     }
 
     if (!dom) { /* list all domain''s credit scheduler info */
-        return -sched_domain_output(XEN_SCHEDULER_SEDF,
+        return -sched_domain_output(LIBXL_SCHEDULER_SEDF,
                                     sched_sedf_domain_output, cpupool);
     } else {
         find_domain(dom);
@@ -5287,9 +5287,8 @@ int main_cpupoolcreate(int argc, char **
     XLU_Config *config;
     const char *buf;
     const char *name;
-    const char *sched;
     uint32_t poolid;
-    int schedid = -1;
+    libxl_scheduler sched = 0;
     XLU_ConfigList *cpus;
     XLU_ConfigList *nodes;
     int n_cpus, n_nodes, i, n;
@@ -5384,17 +5383,16 @@ int main_cpupoolcreate(int argc, char **
     }
 
     if (!xlu_cfg_get_string (config, "sched", &buf, 0)) {
-        if ((schedid = libxl_name_to_schedid(ctx, buf)) < 0) {
+        if ((libxl_scheduler_from_string(buf, &sched)) < 0) {
             fprintf(stderr, "Unknown scheduler\n");
             return -ERROR_FAIL;
         }
     } else {
-        if ((schedid = libxl_get_sched_id(ctx)) < 0) {
-            fprintf(stderr, "get_sched_id sysctl failed.\n");
+        if ((sched = libxl_get_scheduler(ctx)) < 0) {
+            fprintf(stderr, "get_scheduler sysctl failed.\n");
             return -ERROR_FAIL;
         }
     }
-    sched = libxl_schedid_to_name(ctx, schedid);
 
     if (libxl_get_freecpus(ctx, &freemap)) {
         fprintf(stderr, "libxl_get_freecpus failed\n");
@@ -5462,14 +5460,14 @@ int main_cpupoolcreate(int argc, char **
 
     printf("Using config file \"%s\"\n", filename);
     printf("cpupool name:   %s\n", name);
-    printf("scheduler:      %s\n", sched);
+    printf("scheduler:      %s\n", libxl_scheduler_to_string(sched));
     printf("number of cpus: %d\n", n_cpus);
 
     if (dryrun_only)
         return 0;
 
     poolid = 0;
-    if (libxl_cpupool_create(ctx, name, schedid, cpumap, &uuid,
&poolid)) {
+    if (libxl_cpupool_create(ctx, name, sched, cpumap, &uuid, &poolid))
{
         fprintf(stderr, "error on creating cpupool\n");
         return -ERROR_FAIL;
     }
@@ -5554,7 +5552,7 @@ int main_cpupoollist(int argc, char **ar
                     }
                 if (!opt_cpus) {
                     printf("%3d %9s       y       %4d", n,
-                           libxl_schedid_to_name(ctx, poolinfo[p].sched_id),
+                           libxl_scheduler_to_string(poolinfo[p].sched),
                            poolinfo[p].n_dom);
                 }
                 printf("\n");
@@ -5739,7 +5737,7 @@ int main_cpupoolnumasplit(int argc, char
     int c;
     int n;
     uint32_t poolid;
-    int schedid;
+    libxl_scheduler sched;
     int n_pools;
     int node;
     int n_cpus;
@@ -5760,7 +5758,7 @@ int main_cpupoolnumasplit(int argc, char
         return -ERROR_NOMEM;
     }
     poolid = poolinfo[0].poolid;
-    schedid = poolinfo[0].sched_id;
+    sched = poolinfo[0].sched;
     for (p = 0; p < n_pools; p++) {
         libxl_cpupoolinfo_dispose(poolinfo + p);
     }
@@ -5840,7 +5838,7 @@ int main_cpupoolnumasplit(int argc, char
         snprintf(name, 15, "Pool-node%d", node);
         libxl_uuid_generate(&uuid);
         poolid = 0;
-        ret = -libxl_cpupool_create(ctx, name, schedid, cpumap, &uuid,
&poolid);
+        ret = -libxl_cpupool_create(ctx, name, sched, cpumap, &uuid,
&poolid);
         if (ret) {
             fprintf(stderr, "error on creating cpupool\n");
             goto out;

On Thu, 2012-02-23 at 08:45 +0000, Ian Campbell wrote:
> On Wed, 2012-02-22 at 21:40 +0000, Jim Fehlig wrote:
> > Ian Jackson wrote:
> > > Bamvor Jian Zhang writes ("Re: [Xen-devel] [xen-devel]
[PATCH] libxl: fix compile error of libvirt"):
> > >
> > >> Ian Jackson wrote:
> > >>
> > >>> Users of libxl should not be using libxc directly and
therefore should
> > >>> not be including xenctrl.h.
> > >>>
> > > ...
> > >
> > >> but after your commit "23174:751c6dcec0d4"(remove
xenctrl.h from libxl.h), the aplication(like libvirt) compile fail. How do i
deal with it?
> > >> it seems that add __XEN_TOOLS_ to libvirt code is not good.
> > >>
> > >
> > > Can you tell us the error message you get ?  I think the problem
is
> > > probably that libvirt is trying to use libxc directly.
> > >
> >
> > The libvirt libxl driver doesn''t use libxc directly. AFAICT,
the problem
> > is that libxl.h includes <xen/sysctl.h>, which has this
> >
> > #if !defined(__XEN__) && !defined(__XEN_TOOLS__)
> > #error "sysctl operations are intended for use by node control
tools only"
> > #endif
> >
> > Without the defines, Bamvor is hitting the #error directive.
> 
> I thought we''d discussed and resolved this ages ago but I guess we
only
> discussed it...
> 
> How about the following:
I also have the following, I think the benefits are less obvious for
this one though but it does mean we are consistently avoiding all xen
public headers in the libxl public API which at least has the benefit of
being simple to describe.

8<-------------------------------

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329986824 0
# Node ID ad39625c16966fe004c25e5be81af0e32cf8646a
# Parent  d256d6b42ee77cdff0356c37d6fcaf6203a21ba6
libxl: Remove xen/sched.h from public interface

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r d256d6b42ee7 -r ad39625c1696 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Thu Feb 23 08:37:13 2012 +0000
+++ b/tools/libxl/libxl.h	Thu Feb 23 08:47:04 2012 +0000
@@ -137,8 +137,6 @@
 
 #include <xentoollog.h>
 
-#include <xen/sched.h>
-
 #include <libxl_uuid.h>
 #include <_libxl_list.h>
 
@@ -638,12 +636,7 @@ int libxl_cpupool_cpuremove(libxl_ctx *c
 int libxl_cpupool_cpuremove_node(libxl_ctx *ctx, uint32_t poolid, int node, int
*cpus);
 int libxl_cpupool_movedomain(libxl_ctx *ctx, uint32_t poolid, uint32_t domid);
 
-static inline int libxl_domid_valid_guest(uint32_t domid)
-{
-    /* returns 1 if the value _could_ be a valid guest domid, 0 otherwise
-     * does not check whether the domain actually exists */
-    return domid > 0 && domid < DOMID_FIRST_RESERVED;
-}
+int libxl_domid_valid_guest(uint32_t domid);
 
 int libxl_flask_context_to_sid(libxl_ctx *ctx, char *buf, size_t len,
                                uint32_t *ssidref);
diff -r d256d6b42ee7 -r ad39625c1696 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Thu Feb 23 08:37:13 2012 +0000
+++ b/tools/libxl/libxl_types.idl	Thu Feb 23 08:47:04 2012 +0000
@@ -107,6 +107,15 @@ libxl_scheduler = Enumeration("scheduler
     (7, "arinc653"),
     ])
 
+# Consistent with SHUTDOWN_* in sched.h
+libxl_shutdown_reason = Enumeration("shutdown_reason", [
+    (0, "poweroff"),
+    (1, "reboot"),
+    (2, "suspend"),
+    (3, "crash"),
+    (4, "watchdog"),
+    ])
+
 #
 # Complex libxl types
 #
@@ -154,7 +163,7 @@ libxl_dominfo = Struct("dominfo",[
     #
     # Otherwise set to a value guaranteed not to clash with any valid
     # SHUTDOWN_* constant.
-    ("shutdown_reason", uint8),
+    ("shutdown_reason", libxl_shutdown_reason),
     ("current_memkb",   uint64),
     ("shared_memkb", uint64),
     ("max_memkb",   uint64),
diff -r d256d6b42ee7 -r ad39625c1696 tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c	Thu Feb 23 08:37:13 2012 +0000
+++ b/tools/libxl/libxl_utils.c	Thu Feb 23 08:47:04 2012 +0000
@@ -507,6 +507,13 @@ void libxl_cputopology_list_free(libxl_c
     free(list);
 }
 
+int libxl_domid_valid_guest(uint32_t domid)
+{
+    /* returns 1 if the value _could_ be a valid guest domid, 0 otherwise
+     * does not check whether the domain actually exists */
+    return domid > 0 && domid < DOMID_FIRST_RESERVED;
+}
+
 /*
  * Local variables:
  * mode: C
diff -r d256d6b42ee7 -r ad39625c1696 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Thu Feb 23 08:37:13 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Thu Feb 23 08:47:04 2012 +0000
@@ -1230,19 +1230,19 @@ static int handle_domain_death(libxl_ctx
     libxl_action_on_shutdown action;
 
     switch (event->u.domain_shutdown.shutdown_reason) {
-    case SHUTDOWN_poweroff:
+    case LIBXL_SHUTDOWN_REASON_POWEROFF:
         action = d_config->on_poweroff;
         break;
-    case SHUTDOWN_reboot:
+    case LIBXL_SHUTDOWN_REASON_REBOOT:
         action = d_config->on_reboot;
         break;
-    case SHUTDOWN_suspend:
+    case LIBXL_SHUTDOWN_REASON_SUSPEND:
         LOG("Domain has suspended.");
         return 0;
-    case SHUTDOWN_crash:
+    case LIBXL_SHUTDOWN_REASON_CRASH:
         action = d_config->on_crash;
         break;
-    case SHUTDOWN_watchdog:
+    case LIBXL_SHUTDOWN_REASON_WATCHDOG:
         action = d_config->on_watchdog;
         break;
     default:

On Wed, 2012-02-22 at 21:42 +0000, Jim Fehlig wrote:
> Ian Jackson wrote:
> > Note that the API for libxl has changed in xen-unstable.hg compared to
> > 4.1, and further changes are forthcoming.  So there will have to be
> > changes in libvirt.
> >   
> 
> I suppose libvirt is the only out-of-tree libxl app feeling this pain :-(.
We''re sorry about this, we felt it was necessary to break the API
between 4.1 and 4.2 in order that we can support a stable API from 4.2
onwards (I think we discussed this at the time?). From 4.2 onwards we
will make changes in a compatible way or provide compatibility shims etc
or in extreme circumstances do things in a way which is easily
detectable e.g. via configure tests.

I was wondering the other day if "libxl v2" support for libvirt might
make a nice GSoC project, what do you think? Would you be interested in
mentoring such a project?

Ian.

On Thu, 2012-02-23 at 08:52 +0000, Ian Campbell wrote:
> On Thu, 2012-02-23 at 08:45 +0000, Ian Campbell wrote:
> > On Wed, 2012-02-22 at 21:40 +0000, Jim Fehlig wrote:
> > > Ian Jackson wrote:
> > > > Bamvor Jian Zhang writes ("Re: [Xen-devel] [xen-devel]
[PATCH] libxl: fix compile error of libvirt"):
> > > >
> > > >> Ian Jackson wrote:
> > > >>
> > > >>> Users of libxl should not be using libxc directly
and therefore should
> > > >>> not be including xenctrl.h.
> > > >>>
> > > > ...
> > > >
> > > >> but after your commit
"23174:751c6dcec0d4"(remove xenctrl.h from libxl.h), the
aplication(like libvirt) compile fail. How do i deal with it?
> > > >> it seems that add __XEN_TOOLS_ to libvirt code is not
good.
> > > >>
> > > >
> > > > Can you tell us the error message you get ?  I think the
problem is
> > > > probably that libvirt is trying to use libxc directly.
> > > >
> > >
> > > The libvirt libxl driver doesn''t use libxc directly.
AFAICT, the problem
> > > is that libxl.h includes <xen/sysctl.h>, which has this
> > >
> > > #if !defined(__XEN__) && !defined(__XEN_TOOLS__)
> > > #error "sysctl operations are intended for use by node
control tools only"
> > > #endif
> > >
> > > Without the defines, Bamvor is hitting the #error directive.
> > 
> > I thought we''d discussed and resolved this ages ago but I
guess we only
> > discussed it...
> > 
> > How about the following:
> 
> I also have the following,
One or both of which break the bindings build...

I''ll resend as precursor to my resend of the "better defaults
handling".
Simply because there are some textual conflicts between the two.

I''d still be happy to have Acks for the libxl side though (the bindings
changes will be obvious I expect)

Ian.

Ian Campbell wrote:
> On Wed, 2012-02-22 at 21:42 +0000, Jim Fehlig wrote:
>   
>> Ian Jackson wrote:
>>     
>>> Note that the API for libxl has changed in xen-unstable.hg compared
to
>>> 4.1, and further changes are forthcoming.  So there will have to be
>>> changes in libvirt.
>>>   
>>>       
>> I suppose libvirt is the only out-of-tree libxl app feeling this pain
:-(.
>>     
>
> We''re sorry about this, we felt it was necessary to break the API
> between 4.1 and 4.2 in order that we can support a stable API from 4.2
> onwards (I think we discussed this at the time?).
Yes we did, and I need to stop whining about it :-).
>  From 4.2 onwards we
> will make changes in a compatible way or provide compatibility shims etc
> or in extreme circumstances do things in a way which is easily
> detectable e.g. via configure tests.
>   
That would be much appreciated.  Maybe we''ll start seeing more libxl
apps...
> I was wondering the other day if "libxl v2" support for libvirt
might
> make a nice GSoC project, what do you think? Would you be interested in
> mentoring such a project?
>   
I agree that it would make a nice GSoC project, but I had encouraged
Bamvor to take on the task.  It''s a good project for becoming familiar
with xen and libvirt.

Regards,
Jim

On Fri, 2012-02-24 at 03:05 +0000, Jim Fehlig wrote:
> > I was wondering the other day if "libxl v2" support for
libvirt might
> > make a nice GSoC project, what do you think? Would you be interested
in
> > mentoring such a project?
> >   
> 
> I agree that it would make a nice GSoC project, but I had encouraged
> Bamvor to take on the task.  It''s a good project for becoming
familiar
> with xen and libvirt.
> 
Absolutely, good to see Bamvor will be working on it!

Are there any related subprojects or features relating to libvirt+libxl
which might make a good GSoC thing?

Ian.

Jim Fehlig writes ("Re: [Xen-devel] [xen-devel] [PATCH] libxl: fix compile
error of libvirt"):
> The libvirt libxl driver doesn''t use libxc directly. AFAICT, the
problem
> is that libxl.h includes <xen/sysctl.h>, which has this
> 
> #if !defined(__XEN__) && !defined(__XEN_TOOLS__)
> #error "sysctl operations are intended for use by node control tools
only"
> #endif
> 
> Without the defines, Bamvor is hitting the #error directive.
I see.  Hmm.  I have investigated further and:

xl.c contained #include <xenctrl.h> which is definitely very wrong.
when I
 (a) #undef __XEN_TOOLS__ at the top of xl.c
 (b) remove that
I can reproduce the error in-tree.

I''m unsure as to whether we should expect libxl callers include
<xen/sysctl.h>.  I think my view is that including these Xen public
headers for things like Xen sysctl numbers, scheduler parameter
numbers, etc., is fine.  Ian, what do you think ?

But we definitely need to do something to stop libxl callers,
including xl, including xenctrl.h.

Ian.

On Fri, 2012-02-24 at 12:18 +0000, Ian Jackson wrote:
> Jim Fehlig writes ("Re: [Xen-devel] [xen-devel] [PATCH] libxl: fix
compile error of libvirt"):
> > The libvirt libxl driver doesn''t use libxc directly. AFAICT,
the problem
> > is that libxl.h includes <xen/sysctl.h>, which has this
> > 
> > #if !defined(__XEN__) && !defined(__XEN_TOOLS__)
> > #error "sysctl operations are intended for use by node control
tools only"
> > #endif
> > 
> > Without the defines, Bamvor is hitting the #error directive.
> 
> I see.  Hmm.  I have investigated further and:
> 
> xl.c contained #include <xenctrl.h> which is definitely very wrong.
> when I
>  (a) #undef __XEN_TOOLS__ at the top of xl.c
>  (b) remove that
> I can reproduce the error in-tree.
Doing this with the two patches I sent yesterday works. (I resewnt them
at the head of my repost of the "libxl: improved handling for default
values in API" series.

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?
> I''m unsure as to whether we should expect libxl callers include
> <xen/sysctl.h>.  I think my view is that including these Xen public
> headers for things like Xen sysctl numbers, scheduler parameter
> numbers, etc., is fine.  Ian, what do you think ?
> 
> But we definitely need to do something to stop libxl callers,
> including xl, including xenctrl.h.
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.

Ian.

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@eu.citrix.com>
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@eu.citrix.com>

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

(I could have sworn I hit send on this. apologies if you get it twice
somehow)

On Fri, 2012-02-24 at 15:42 +0000, Ian Jackson wrote:
> 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.
It''s clever, some might say too clever ;-)

I''m a bit concerned about the use of domctl and sysctl in the libxl API
anyway -- I''m not convinced those are guaranteed to be stable ABIs by
the hypervisor (in fact, I''m reasonably sure they are not -- hence the
#error), in which case we mustn''t expose them to libxl''s
users.

sched.h concerns me less -- that one is guest visible API IIRC and
therefore stable. But sched.h doesn''t have a #error in it so
isn''t a
problem.

So I think we have to take the first of my patches. At which point we
can avoid the twisty maze of your patch using -U... as necessary.

Ian.
> 
> Ian.
> 
> From: Ian Jackson <ian.jackson@eu.citrix.com>
> 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@eu.citrix.com>
> 
> ---
>  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"