[PATCH] change dom0 headers path

[PATCH] change dom0 headers path

[John Levon]

Fairly obviously, we need to clean up the hard-coded "linux" names used
in includes. This is the first step in a number of changes around
letting dom0 build on something other than Linux.

What are the plans for these headers when the Linux kernel lives in some
place other than the xen tree itself? We'd like to be able to share the
structure definitions, but our ioctl values will differ from Linux's...

regards,
john

# HG changeset patch
# User john.levon@sun.com
# Node ID 0261b8ddaa8ae466b5cb7e5732fabc1fd25bc73c
# Parent  72f9c751d3ea1f17ff513cd7fc2cbe671a9af7c9
Put dom0 headers used by the tools under xen/dom0/ instead of xen/linux/.

Signed-off-by: John Levon <john.levon@sun.com>

diff -r 72f9c751d3ea -r 0261b8ddaa8a tools/Rules.mk
--- a/tools/Rules.mk	Wed Apr 19 18:32:20 2006 +0100
+++ b/tools/Rules.mk	Wed Apr 19 11:37:30 2006 -0700
@@ -23,15 +23,22 @@
 %.o: %.cc
 	$(CC) $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $<
 
-.PHONY: mk-symlinks
-mk-symlinks: LINUX_ROOT=$(XEN_ROOT)/linux-2.6-xen-sparse
-mk-symlinks:
+OS = $(shell uname -s)
+
+.PHONY: mk-symlinks mk-symlinks-$(OS) mk-symlinks-xen
+
+mk-symlinks-Linux: LINUX_ROOT=$(XEN_ROOT)/linux-2.6-xen-sparse
+mk-symlinks-Linux:
+	mkdir -p xen/dom0
+	( cd xen/dom0 && \
+	  ln -sf ../../$(LINUX_ROOT)/include/xen/public/*.h . )
+
+mk-symlinks-xen:
 	mkdir -p xen
 	( cd xen && ln -sf ../$(XEN_ROOT)/xen/include/public/*.h . )
 	mkdir -p xen/hvm
 	( cd xen/hvm && ln -sf ../../$(XEN_ROOT)/xen/include/public/hvm/*.h . )
 	mkdir -p xen/io
 	( cd xen/io && ln -sf ../../$(XEN_ROOT)/xen/include/public/io/*.h . )
-	mkdir -p xen/linux
-	( cd xen/linux && \
-	  ln -sf ../../$(LINUX_ROOT)/include/xen/public/*.h . )
+
+mk-symlinks: mk-symlinks-xen mk-symlinks-$(OS)
diff -r 72f9c751d3ea -r 0261b8ddaa8a tools/console/daemon/io.c
--- a/tools/console/daemon/io.c	Wed Apr 19 18:32:20 2006 +0100
+++ b/tools/console/daemon/io.c	Wed Apr 19 11:37:30 2006 -0700
@@ -24,7 +24,7 @@
 #include "io.h"
 #include <xenctrl.h>
 #include <xs.h>
-#include <xen/linux/evtchn.h>
+#include <xen/dom0/evtchn.h>
 #include <xen/io/console.h>
 
 #include <malloc.h>
diff -r 72f9c751d3ea -r 0261b8ddaa8a tools/debugger/pdb/pdb_caml_process.c
--- a/tools/debugger/pdb/pdb_caml_process.c	Wed Apr 19 18:32:20 2006 +0100
+++ b/tools/debugger/pdb/pdb_caml_process.c	Wed Apr 19 11:37:30 2006 -0700
@@ -18,7 +18,7 @@
 #include <xenctrl.h>
 #include <xen/xen.h>
 #include <xen/io/domain_controller.h>
-#include <xen/linux/privcmd.h>
+#include <xen/dom0/privcmd.h>
 #include "pdb_module.h"
 #include "pdb_caml_xen.h"
 
diff -r 72f9c751d3ea -r 0261b8ddaa8a tools/debugger/pdb/pdb_caml_xcs.c
--- a/tools/debugger/pdb/pdb_caml_xcs.c	Wed Apr 19 18:32:20 2006 +0100
+++ b/tools/debugger/pdb/pdb_caml_xcs.c	Wed Apr 19 11:37:30 2006 -0700
@@ -21,7 +21,7 @@
 
 #include <xen/xen.h>
 #include <xen/io/domain_controller.h>
-#include <xen/linux/privcmd.h>
+#include <xen/dom0/privcmd.h>
 
 #include <arpa/inet.h>
 #include <xcs_proto.h>
diff -r 72f9c751d3ea -r 0261b8ddaa8a tools/debugger/pdb/pdb_xen.c
--- a/tools/debugger/pdb/pdb_xen.c	Wed Apr 19 18:32:20 2006 +0100
+++ b/tools/debugger/pdb/pdb_xen.c	Wed Apr 19 11:37:30 2006 -0700
@@ -43,7 +43,7 @@
 
 
 #include <sys/ioctl.h>
-#include <xen/linux/evtchn.h>
+#include <xen/dom0/evtchn.h>
 
 int
 xen_evtchn_bind (int evtchn_fd, int idx)
diff -r 72f9c751d3ea -r 0261b8ddaa8a tools/ioemu/target-i386-dm/helper2.c
--- a/tools/ioemu/target-i386-dm/helper2.c	Wed Apr 19 18:32:20 2006 +0100
+++ b/tools/ioemu/target-i386-dm/helper2.c	Wed Apr 19 11:37:30 2006 -0700
@@ -51,7 +51,7 @@
 
 #include <xenctrl.h>
 #include <xen/hvm/ioreq.h>
-#include <xen/linux/evtchn.h>
+#include <xen/dom0/evtchn.h>
 
 #include "cpu.h"
 #include "exec-all.h"
diff -r 72f9c751d3ea -r 0261b8ddaa8a tools/libxc/xc_private.h
--- a/tools/libxc/xc_private.h	Wed Apr 19 18:32:20 2006 +0100
+++ b/tools/libxc/xc_private.h	Wed Apr 19 11:37:30 2006 -0700
@@ -15,7 +15,7 @@
 
 #include "xenctrl.h"
 
-#include <xen/linux/privcmd.h>
+#include <xen/dom0/privcmd.h>
 
 /* valgrind cannot see when a hypercall has filled in some values.  For this
    reason, we must zero the privcmd_hypercall_t or dom0_op_t instance before a
diff -r 72f9c751d3ea -r 0261b8ddaa8a tools/libxc/xg_private.h
--- a/tools/libxc/xg_private.h	Wed Apr 19 18:32:20 2006 +0100
+++ b/tools/libxc/xg_private.h	Wed Apr 19 11:37:30 2006 -0700
@@ -13,7 +13,7 @@
 #include "xenctrl.h"
 #include "xenguest.h"
 
-#include <xen/linux/privcmd.h>
+#include <xen/dom0/privcmd.h>
 #include <xen/memory.h>
 
 /* valgrind cannot see when a hypercall has filled in some values.  For this
diff -r 72f9c751d3ea -r 0261b8ddaa8a tools/security/get_decision.c
--- a/tools/security/get_decision.c	Wed Apr 19 18:32:20 2006 +0100
+++ b/tools/security/get_decision.c	Wed Apr 19 11:37:30 2006 -0700
@@ -30,7 +30,7 @@
 #include <netinet/in.h>
 #include <xen/acm.h>
 #include <xen/acm_ops.h>
-#include <xen/linux/privcmd.h>
+#include <xen/dom0/privcmd.h>
 
 #define PERROR(_m, _a...) \
 fprintf(stderr, "ERROR: " _m " (%d = %s)\n" , ## _a ,	\
diff -r 72f9c751d3ea -r 0261b8ddaa8a tools/security/secpol_tool.c
--- a/tools/security/secpol_tool.c	Wed Apr 19 18:32:20 2006 +0100
+++ b/tools/security/secpol_tool.c	Wed Apr 19 11:37:30 2006 -0700
@@ -36,7 +36,7 @@
 #include <stdint.h>
 #include <xen/acm.h>
 #include <xen/acm_ops.h>
-#include <xen/linux/privcmd.h>
+#include <xen/dom0/privcmd.h>
 
 #define PERROR(_m, _a...) \
 fprintf(stderr, "ERROR: " _m " (%d = %s)\n" , ## _a ,	\
diff -r 72f9c751d3ea -r 0261b8ddaa8a tools/xenmon/xenbaked.c
--- a/tools/xenmon/xenbaked.c	Wed Apr 19 18:32:20 2006 +0100
+++ b/tools/xenmon/xenbaked.c	Wed Apr 19 11:37:30 2006 -0700
@@ -44,7 +44,7 @@
 #include <xen/xen.h>
 #include <string.h>
 #include <sys/select.h>
-#include <xen/linux/evtchn.h>
+#include <xen/dom0/evtchn.h>
 
 #include "xc_private.h"
 typedef struct { int counter; } atomic_t;
diff -r 72f9c751d3ea -r 0261b8ddaa8a tools/xenstat/libxenstat/src/xen-interface.c
--- a/tools/xenstat/libxenstat/src/xen-interface.c	Wed Apr 19 18:32:20 2006 +0100
+++ b/tools/xenstat/libxenstat/src/xen-interface.c	Wed Apr 19 11:37:30 2006 -0700
@@ -23,7 +23,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
-#include <xen/linux/privcmd.h>
+#include <xen/dom0/privcmd.h>
 
 struct xi_handle {
 	int fd;
diff -r 72f9c751d3ea -r 0261b8ddaa8a tools/xenstore/xenstored_domain.c
--- a/tools/xenstore/xenstored_domain.c	Wed Apr 19 18:32:20 2006 +0100
+++ b/tools/xenstore/xenstored_domain.c	Wed Apr 19 11:37:30 2006 -0700
@@ -38,7 +38,7 @@
 #include "xenstored_test.h"
 
 #include <xenctrl.h>
-#include <xen/linux/evtchn.h>
+#include <xen/dom0/evtchn.h>
 
 static int *xc_handle;
 static evtchn_port_t virq_port;

Re: [PATCH] change dom0 headers path

[Keir Fraser]

On 20 Apr 2006, at 13:39, John Levon wrote:

> Fairly obviously, we need to clean up the hard-coded "linux" names used
> in includes. This is the first step in a number of changes around
> letting dom0 build on something other than Linux.
>
> What are the plans for these headers when the Linux kernel lives in 
> some
> place other than the xen tree itself? We'd like to be able to share the
> structure definitions, but our ioctl values will differ from Linux's...

I'd prefer an interfacing library (or libraries) that can target Linux 
interfaces, Sun interfaces, etc. This would avoid inclusion of kernel 
interface headers outside that shim library.

Maybe this only needs to be done for evtchn interfaces. I would have 
hoped that libxenctrl and libxenguest would hide privcmd interfaces. I 
wonder why so many things include <linux/privcmd.h>?

  -- Keir

Re: [PATCH] change dom0 headers path

[John Levon]

On Thu, Apr 20, 2006 at 02:00:07PM +0100, Keir Fraser wrote:

> >Fairly obviously, we need to clean up the hard-coded "linux" names used
> >in includes. This is the first step in a number of changes around
> >letting dom0 build on something other than Linux.
> >
> >What are the plans for these headers when the Linux kernel lives in 
> >some
> >place other than the xen tree itself? We'd like to be able to share the
> >structure definitions, but our ioctl values will differ from Linux's...
> 
> I'd prefer an interfacing library (or libraries) that can target Linux 
> interfaces, Sun interfaces, etc.

My understanding is that libxc *is* that library.

> Maybe this only needs to be done for evtchn interfaces. I would have 
> hoped that libxenctrl and libxenguest would hide privcmd interfaces. I 
> wonder why so many things include <linux/privcmd.h>?

Code is using ioctl() where it should be using a helper function.  I
don't mind looking into cleaning these things up at some point, but it
doesn't seem critical right now. But we'd like to get a firm grasp on
header naming so we can deal with the unfortunate two-way dependency
these headers have between dom0 and dom0 userspace.

IOW, I agree with you, but I think the patch needs to go in regardless.
In particular, something like my patch will still be needed, even if
it's just private to tools/libxc/.

regards,
john

Re: [PATCH] change dom0 headers path

[Keir Fraser]

On 20 Apr 2006, at 14:13, John Levon wrote:

> Code is using ioctl() where it should be using a helper function.  I
> don't mind looking into cleaning these things up at some point, but it
> doesn't seem critical right now. But we'd like to get a firm grasp on
> header naming so we can deal with the unfortunate two-way dependency
> these headers have between dom0 and dom0 userspace.
>
> IOW, I agree with you, but I think the patch needs to go in regardless.
> In particular, something like my patch will still be needed, even if
> it's just private to tools/libxc/.

I'd have a solaris header subdirectory in addition to the linux one. 
The interfaces may not necessarily stay very similar.

  -- Keir

Re: [PATCH] change dom0 headers path

[John Levon]

On Thu, Apr 20, 2006 at 02:36:57PM +0100, Keir Fraser wrote:

> >Code is using ioctl() where it should be using a helper function.  I
> >don't mind looking into cleaning these things up at some point, but it
> >doesn't seem critical right now. But we'd like to get a firm grasp on
> >header naming so we can deal with the unfortunate two-way dependency
> >these headers have between dom0 and dom0 userspace.
> >
> >IOW, I agree with you, but I think the patch needs to go in regardless.
> >In particular, something like my patch will still be needed, even if
> >it's just private to tools/libxc/.
> 
> I'd have a solaris header subdirectory in addition to the linux one. 
> The interfaces may not necessarily stay very similar.

That's exactly what the patch as-is allows for: libxc/xen/dom0/ is only
created on Linux. If you'd prefer me to always create libxc/xen/linux/,
then make a dom0/ symlink point at it, that's fine, but it's kind of
pointless since it wouldn't get used on non-Linux anyway. It was also
one reason I was asking about plans for when linux sparse tree doesn't
exist.

Or maybe I've got the wrong end of the stick; what are you proposing?

Note that I'm not at all sure that it makes sense to have the Solaris
headers in there in xensource's tree as Xen and the kernel have such
different distribution mechanisms.

What we'd /really/ like to have, in order to maximise the shared code,
is a permanent file in libxc/dom0/ containing the structures, which
includes a kernel-specific header for the actual ioctl defines, and any
future kernel-specific bits.

regards,
john

Re: [PATCH] change dom0 headers path

[Anthony Liguori]

Keir Fraser wrote:
>
> On 20 Apr 2006, at 13:39, John Levon wrote:
>
>> Fairly obviously, we need to clean up the hard-coded "linux" names used
>> in includes. This is the first step in a number of changes around
>> letting dom0 build on something other than Linux.
>>
>> What are the plans for these headers when the Linux kernel lives in some
>> place other than the xen tree itself? We'd like to be able to share the
>> structure definitions, but our ioctl values will differ from Linux's...
>
> I'd prefer an interfacing library (or libraries) that can target Linux 
> interfaces, Sun interfaces, etc. This would avoid inclusion of kernel 
> interface headers outside that shim library.
>
> Maybe this only needs to be done for evtchn interfaces. I would have 
> hoped that libxenctrl and libxenguest would hide privcmd interfaces. I 
> wonder why so many things include <linux/privcmd.h>?

Because libxc "leaks" a few of the dom0_op structures like 
dom0_getvcpuinfo_t, dom0_shadow_control_stats_t, etc in its public 
interface.  This could be fixed by making sure all of those structures 
had wrapper (like xc_dominfo_t).

Regards,

Anthony Liguori

>  -- Keir
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] change dom0 headers path

[Keir Fraser]

On 20 Apr 2006, at 14:55, Anthony Liguori wrote:

> Because libxc "leaks" a few of the dom0_op structures like 
> dom0_getvcpuinfo_t, dom0_shadow_control_stats_t, etc in its public 
> interface.  This could be fixed by making sure all of those structures 
> had wrapper (like xc_dominfo_t).

Those are defined in Xen public header files, not in Linux's privcmd.h.

  -- Keir

Re: [PATCH] change dom0 headers path

[Keir Fraser]

On 20 Apr 2006, at 14:47, John Levon wrote:

> That's exactly what the patch as-is allows for: libxc/xen/dom0/ is only
> created on Linux. If you'd prefer me to always create libxc/xen/linux/,
> then make a dom0/ symlink point at it, that's fine, but it's kind of
> pointless since it wouldn't get used on non-Linux anyway. It was also
> one reason I was asking about plans for when linux sparse tree doesn't
> exist.
>
> Or maybe I've got the wrong end of the stick; what are you proposing?

Well, it depends how much commonality we think there will be between 
different OS's header files in future. If the details are hidden inside 
libxc then it wouldn't hurt to guarantee very little at the kernel and 
have libxc include from linux/ or solaris/ directly depending on how it 
is targetted at compile time. I'm not keen on the name dom0/ anyway -- 
those headers (particularly evtchn.h) are applicable to other domains 
too.

  -- Keir

Re: [PATCH] change dom0 headers path

[John Levon]

On Thu, Apr 20, 2006 at 03:31:09PM +0100, Keir Fraser wrote:

> >That's exactly what the patch as-is allows for: libxc/xen/dom0/ is only
> >created on Linux. If you'd prefer me to always create libxc/xen/linux/,
> >then make a dom0/ symlink point at it, that's fine, but it's kind of
> >pointless since it wouldn't get used on non-Linux anyway. It was also
> >one reason I was asking about plans for when linux sparse tree doesn't
> >exist.
> >
> >Or maybe I've got the wrong end of the stick; what are you proposing?
> 
> Well, it depends how much commonality we think there will be between 
> different OS's header files in future. 

I suppose it also depends on the future of the ioctl() interface in
Linux, as Anthony pointed out to me. To a large degree how much sharing
can really happen depends strongly on how these interfaces end up. We
already have some differences in the xenbus driver to replace
/proc/xen/xsd_{kva,port}.

> If the details are hidden inside libxc then it wouldn't hurt to
> guarantee very little at the kernel and have libxc include from linux/
> or solaris/ directly depending on how it is targetted at compile time.
> I'm not keen on the name dom0/ anyway -- those headers (particularly
> evtchn.h) are applicable to other domains too.

So, something more like:

libxc/xc_linux.c
libxc/xc_solaris.c
libxc/xen/linux/
libxc/xen/solaris/

with:

1) all the stuff hidden back into libxc
2) Makefile gubbins to -I the right path and pick the right C file

This seems workable for us.

regards,
john

Re: [PATCH] change dom0 headers path

[Anthony Liguori]

Keir Fraser wrote:
>
> On 20 Apr 2006, at 14:55, Anthony Liguori wrote:
>
>> Because libxc "leaks" a few of the dom0_op structures like 
>> dom0_getvcpuinfo_t, dom0_shadow_control_stats_t, etc in its public 
>> interface.  This could be fixed by making sure all of those 
>> structures had wrapper (like xc_dominfo_t).
>
> Those are defined in Xen public header files, not in Linux's privcmd.h.

Yes, sorry, I read the first mail to quickly.  This is another of my 
gripes though (it would be nice if xenctrl.h didn't pull in the Xen 
public headers too).

About privcmd.h, the users of it are making hypercalls that aren't 
exposed through libxenctrl.  Those users probably ought to add the 
appropriate plumbing to libxenctrl.  This includes the ACM stuff, 
xenmon/xentrace, and xenstat.

Regards,

Anthony Liguori

>  -- Keir
>

Re: [PATCH] change dom0 headers path

[Keir Fraser]

On 20 Apr 2006, at 15:40, John Levon wrote:

> So, something more like:
>
> libxc/xc_linux.c
> libxc/xc_solaris.c
> libxc/xen/linux/
> libxc/xen/solaris/
>
> with:
>
> 1) all the stuff hidden back into libxc
> 2) Makefile gubbins to -I the right path and pick the right C file
>
> This seems workable for us.

This is my preference. Of course it is dependent on noone outside 
libxc/ including the OS-specific headers directly, so that will need 
cleaning up. That's a reasonable thing to do though.

  -- Keir

Re: [PATCH] change dom0 headers path

[Keir Fraser]

On 20 Apr 2006, at 15:43, Anthony Liguori wrote:

> Yes, sorry, I read the first mail to quickly.  This is another of my 
> gripes though (it would be nice if xenctrl.h didn't pull in the Xen 
> public headers too).

Alternatives are to place a copy of the headers in tools space (a bit 
pointless while it shares a repository with Xen, but easy to do if it 
moves to a separate one) or add wrapper structures for everything (a 
pain in the ass for not much benefit that I can see).

> About privcmd.h, the users of it are making hypercalls that aren't 
> exposed through libxenctrl.  Those users probably ought to add the 
> appropriate plumbing to libxenctrl.  This includes the ACM stuff, 
> xenmon/xentrace, and xenstat.

I strongly agree.

  -- Keir

Re: [PATCH] change dom0 headers path

[John Levon]

On Thu, Apr 20, 2006 at 03:40:37PM +0100, John Levon wrote:

> So, something more like:
> 
> libxc/xc_linux.c
> libxc/xc_solaris.c
> libxc/xen/linux/
> libxc/xen/solaris/

Hmm. Looking at this, we've got an awful lot of duplicated code between
xc_{linux,solaris}.c if we only let those files see the kernel headers
containing privcmd_hypercall_t etc.

Wouldn't it be better to have a libxc/xen/sys/[1] symlink so that
xc_private.c and friends can continue to include privcmd_hypercall etc?
That way the contents of xc_{linux,solaris}.c can contain only the parts
that we'll genuinely differ by (use of /proc etc.).

regards
john

[1] or some other name since 'dom0' won't do

Re: [PATCH] change dom0 headers path

[Keir Fraser]

On 21 Apr 2006, at 18:55, John Levon wrote:

> Hmm. Looking at this, we've got an awful lot of duplicated code between
> xc_{linux,solaris}.c if we only let those files see the kernel headers
> containing privcmd_hypercall_t etc.
>
> Wouldn't it be better to have a libxc/xen/sys/[1] symlink so that
> xc_private.c and friends can continue to include privcmd_hypercall etc?
> That way the contents of xc_{linux,solaris}.c can contain only the 
> parts
> that we'll genuinely differ by (use of /proc etc.).

Yes, I think that's okay. sys seems a good choice.

  -- Keir