Hi! Attached patch makes ioemu stubdom build on NetBSD. Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger, le Mon 18 Jan 2010 11:42:43 +0100, a écrit :> --- a/Makefile.target > +++ b/Makefile.target > +ifdef CONFIG_STUBDOM > +CFLAGS += -I$(MINI_OS-ROOT)/include > +endifIsn''t that already done by the stubdom/ Makefile? Or put another way, why is it needed on netbsd when it is not on linux?> --- a/block-vbd.c > +++ b/block-vbd.c > -#include <xenbus.h> > -#include <blkfront.h> > +#include <mini-os/xenbus.h> > +#include <mini-os/blkfront.h>Such changes are probably a good thing, please submit them in a separate patch so they can be applied already.> --- a/hw/xen_machine_fv.c > +++ b/hw/xen_machine_fv.c > @@ -40,8 +40,11 @@ > + > +#ifndef CONFIG_STUBDOM /* defined in <mini-os/x86/os.h> */ > #define test_bit(bit,map) \ > (!!((map)[(bit)/BITS_PER_LONG] & (1UL << ((bit)%BITS_PER_LONG)))) > +#endifSame question: how is it that it is not needed on linux?> index 50dfb6b..1a6e445 100644 > --- a/qemu-common.h > +++ b/qemu-common.h > @@ -9,6 +9,8 @@ > +#include "config-host.h"Why?> --- a/vl.c > +++ b/vl.c > @@ -42,6 +42,7 @@ > +#include "dma.h"Why?> --- a/vnc.c > +++ b/vnc.c > @@ -32,8 +32,8 @@ > -#ifdef CONFIG_STUBDOM > +#if defined(CONFIG_STUBDOM) && defined(__Linux__)I do not understand these. netfront.h is not linux-specific.> -#ifndef CONFIG_STUBDOM > +#if !defined(CONFIG_STUBDOM) || defined(__NetBSD__)I do not understand these either. Stub domains do _not_ have a working SO_REUSEADDR.> index fcf60c3..88f84cd 100755 > --- a/xen-setup-stubdom > +++ b/xen-setup-stubdom > @@ -36,7 +36,15 @@ cat <<END >config-host.h.new > #define CONFIG_QEMU_SHAREDIR "${SHAREDIR}/xen/qemu" > #define HOST_I386 1 > #define HOST_LONG_BITS 32 > +#ifdef __Linux__ > #define HAVE_BYTESWAP_H 1 > +#endif > +#ifdef __NetBSD__ > +#define _BSD 1 > +#define HAVE_MACHINE_BSWAP_H 1 > +#define HAVE_IOVEC 1 > +#define O_LARGEFILE 0 > +#endifI''m not sure about that either. Do you realize that stubdomains are not running linux or BSD but MiniOS?> @@ -358,5 +358,6 @@ int xenfb_pv_display_init(DisplayState *ds) > int xenfb_pv_display_vram(void *data) > { > vga_vram = data; > + return 0; > }Probably better just making it return void. Samuel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Monday 18 January 2010 12:11:05 Samuel Thibault wrote:> Christoph Egger, le Mon 18 Jan 2010 11:42:43 +0100, a écrit : > > --- a/Makefile.target > > +++ b/Makefile.target > > +ifdef CONFIG_STUBDOM > > +CFLAGS += -I$(MINI_OS-ROOT)/include > > +endif > > Isn''t that already done by the stubdom/ Makefile? Or put another way, > why is it needed on netbsd when it is not on linux?To make MiniOS and Stubdom build on NetBSD, I have to restructure the headers. This work is not yet 100% complete but so far I can tell, with the restructuring done, you also need it on Linux.> > > --- a/block-vbd.c > > +++ b/block-vbd.c > > -#include <xenbus.h> > > -#include <blkfront.h> > > +#include <mini-os/xenbus.h> > > +#include <mini-os/blkfront.h> > > Such changes are probably a good thing,They are mandatory to not break the build also on Linux with the restructured headers.> please submit them in a separate patch so they can be applied already.Done. See attachment. Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>> > --- a/hw/xen_machine_fv.c > > +++ b/hw/xen_machine_fv.c > > @@ -40,8 +40,11 @@ > > + > > +#ifndef CONFIG_STUBDOM /* defined in <mini-os/x86/os.h> */ > > #define test_bit(bit,map) \ > > (!!((map)[(bit)/BITS_PER_LONG] & (1UL << ((bit)%BITS_PER_LONG)))) > > +#endif > > Same question: how is it that it is not needed on linux?The actual question here is: Why does gcc on Linux not barf about redeclaration?> > index 50dfb6b..1a6e445 100644 > > --- a/qemu-common.h > > +++ b/qemu-common.h > > @@ -9,6 +9,8 @@ > > +#include "config-host.h" > > Why?To get the right socket declarations and fix loop inclusion of libc and minios sockets.> > --- a/vl.c > > +++ b/vl.c > > @@ -42,6 +42,7 @@ > > +#include "dma.h" > > Why?Fixes warning about missing prototypes (i.e. for dma_helper_init)> > --- a/vnc.c > > +++ b/vnc.c > > @@ -32,8 +32,8 @@ > > -#ifdef CONFIG_STUBDOM > > +#if defined(CONFIG_STUBDOM) && defined(__Linux__) > > I do not understand these. netfront.h is not linux-specific.netfront.h belongs to lwip. What is wrong with using libc ?> > -#ifndef CONFIG_STUBDOM > > +#if !defined(CONFIG_STUBDOM) || defined(__NetBSD__) > > I do not understand these either. Stub domains do _not_ have > a working SO_REUSEADDR.Same here: What is wrong with using libc ?> > index fcf60c3..88f84cd 100755 > > --- a/xen-setup-stubdom > > +++ b/xen-setup-stubdom > > @@ -36,7 +36,15 @@ cat <<END >config-host.h.new > > #define CONFIG_QEMU_SHAREDIR "${SHAREDIR}/xen/qemu" > > #define HOST_I386 1 > > #define HOST_LONG_BITS 32 > > +#ifdef __Linux__ > > #define HAVE_BYTESWAP_H 1 > > +#endif > > +#ifdef __NetBSD__ > > +#define _BSD 1 > > +#define HAVE_MACHINE_BSWAP_H 1 > > +#define HAVE_IOVEC 1 > > +#define O_LARGEFILE 0 > > +#endif > > I''m not sure about that either. Do you realize that stubdomains are not > running linux or BSD but MiniOS?This change is about *building* stubdom on Linux or BSD.> > @@ -358,5 +358,6 @@ int xenfb_pv_display_init(DisplayState *ds) > > int xenfb_pv_display_vram(void *data) > > { > > vga_vram = data; > > + return 0; > > } > > Probably better just making it return void.I don''t see the return code anywhere in use. So yes, making it return void is ok. Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 19/01/2010 09:25, "Christoph Egger" <Christoph.Egger@amd.com> wrote:>> please submit them in a separate patch so they can be applied already. > > Done. See attachment. > > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>Should this stuff be going in for Xen 4.0.0? It''s getting late and I don''t think it''s a feature regression. If this patch is a non-controversial obvious cleanup then that''s probably okay. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Tuesday 19 January 2010 10:37:46 Keir Fraser wrote:> On 19/01/2010 09:25, "Christoph Egger" <Christoph.Egger@amd.com> wrote: > >> please submit them in a separate patch so they can be applied already. > > > > Done. See attachment. > > > > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> > > Should this stuff be going in for Xen 4.0.0? It''s getting late and I don''t > think it''s a feature regression.No, it isn''t.> If this patch is a non-controversial > obvious cleanup then that''s probably okay.For Xen 4.0.0 you can see it as a cleanup. Note, this patch is for ioemu. So it is up to Ian Jackson to apply it. Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger, le Tue 19 Jan 2010 10:25:10 +0100, a écrit :> On Monday 18 January 2010 12:11:05 Samuel Thibault wrote: > > Christoph Egger, le Mon 18 Jan 2010 11:42:43 +0100, a écrit : > > > --- a/Makefile.target > > > +++ b/Makefile.target > > > +ifdef CONFIG_STUBDOM > > > +CFLAGS += -I$(MINI_OS-ROOT)/include > > > +endif > > > > Isn''t that already done by the stubdom/ Makefile? Or put another way, > > why is it needed on netbsd when it is not on linux? > > To make MiniOS and Stubdom build on NetBSD, I have to restructure the headers. > This work is not yet 100% complete but so far I can tell, with the > restructuring done, you also need it on Linux.I guess Keir could prefer to see both patches submitted at the same time. What I don''t understand is why this hunk is needed when at the same time you are adding mini-os/ in include paths, which will permit to just add to -isystem a directory containing a mini-os symlink to MiniOS''s headers, to avoid polluting the include namespace with mini-os headers like this hunk does.> > > --- a/hw/xen_machine_fv.c > > > +++ b/hw/xen_machine_fv.c > > > @@ -40,8 +40,11 @@ > > > + > > > +#ifndef CONFIG_STUBDOM /* defined in <mini-os/x86/os.h> */ > > > #define test_bit(bit,map) \ > > > (!!((map)[(bit)/BITS_PER_LONG] & (1UL << ((bit)%BITS_PER_LONG)))) > > > +#endif > > > > Same question: how is it that it is not needed on linux? > > The actual question here is: Why does gcc on Linux not barf about > redeclaration?It does, but IIRC not about this one, that''s why I''m asking.> > > --- a/vl.c > > > +++ b/vl.c > > > @@ -42,6 +42,7 @@ > > > +#include "dma.h" > > > > Why? > > Fixes warning about missing prototypes (i.e. for dma_helper_init)Doesn''t it impact upstream too? It then should be submitted upstream instead. Ian prefers to keep the diff with upstream as low as possible, don''t add noise to it :)> > > index 50dfb6b..1a6e445 100644 > > > --- a/qemu-common.h > > > +++ b/qemu-common.h > > > @@ -9,6 +9,8 @@ > > > +#include "config-host.h" > > > > Why? > > To get the right socket declarations and fix loop inclusion of > libc and minios sockets.See below.> > > --- a/vnc.c > > > +++ b/vnc.c > > > @@ -32,8 +32,8 @@ > > > -#ifdef CONFIG_STUBDOM > > > +#if defined(CONFIG_STUBDOM) && defined(__Linux__) > > > > I do not understand these. netfront.h is not linux-specific. > > netfront.h belongs to lwip. What is wrong with using libc ?No. netfront.h belongs to MiniOS. Is your libc able to use netfront to be able to send/receive network packets? We already discussed about it on xen-devel. Ian, Stefano and I agreed that we do not want to maintain several kinds of libc to be used in stub domains. They are already difficult enough to debug that we don''t want to have several environment with varying kinds of bugs. You said there were erroneous linuxish assumption currently done. Fine, please show them, and fixing them will be useful for all ports, not only NetBSD.> > > -#ifndef CONFIG_STUBDOM > > > +#if !defined(CONFIG_STUBDOM) || defined(__NetBSD__) > > > > I do not understand these either. Stub domains do _not_ have > > a working SO_REUSEADDR. > > Same here: What is wrong with using libc ?Same answer.> > > index fcf60c3..88f84cd 100755 > > > --- a/xen-setup-stubdom > > > +++ b/xen-setup-stubdom > > > @@ -36,7 +36,15 @@ cat <<END >config-host.h.new > > > #define CONFIG_QEMU_SHAREDIR "${SHAREDIR}/xen/qemu" > > > #define HOST_I386 1 > > > #define HOST_LONG_BITS 32 > > > +#ifdef __Linux__ > > > #define HAVE_BYTESWAP_H 1 > > > +#endif > > > +#ifdef __NetBSD__ > > > +#define _BSD 1 > > > +#define HAVE_MACHINE_BSWAP_H 1 > > > +#define HAVE_IOVEC 1 > > > +#define O_LARGEFILE 0 > > > +#endif > > > > I''m not sure about that either. Do you realize that stubdomains are not > > running linux or BSD but MiniOS? > > This change is about *building* stubdom on Linux or BSD.No. It''s about *cross-building* stubdom on Linux or BSD. MiniOS _does_ have byteswap.h, newlib doesn''t have iovec, etc. etc. Maybe your libc doesn''t/does, but see above. Samuel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger, le Tue 19 Jan 2010 10:25:10 +0100, a écrit :> Done. See attachment. > > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>Acked-By: Samuel Thibault <samuel.thibault@ens-lyon.org> diff --git a/block-vbd.c b/block-vbd.c index 56794f6..11ac4d0 100644 --- a/block-vbd.c +++ b/block-vbd.c @@ -26,8 +26,8 @@ #include "sys-queue.h" #include "block_int.h" #include <assert.h> -#include <xenbus.h> -#include <blkfront.h> +#include <mini-os/xenbus.h> +#include <mini-os/blkfront.h> #include <malloc.h> #include "qemu-char.h" diff --git a/qemu-lock.h b/qemu-lock.h index 6778eea..3a01ebe 100644 --- a/qemu-lock.h +++ b/qemu-lock.h @@ -34,7 +34,7 @@ #elif defined(CONFIG_STUBDOM) -#include <spinlock.h> +#include <mini-os/spinlock.h> #else diff --git a/xenfbfront.c b/xenfbfront.c index ed79cde..ec1f309 100644 --- a/xenfbfront.c +++ b/xenfbfront.c @@ -1,9 +1,9 @@ #include <stdint.h> #include <xen/io/fbif.h> #include <xen/io/kbdif.h> -#include <semaphore.h> -#include <sched.h> -#include <fbfront.h> +#include <mini-os/semaphore.h> +#include <mini-os/sched.h> +#include <mini-os/fbfront.h> #include <hw/hw.h> #include <hw/pc.h> #include <console.h> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser writes ("Re: [Xen-devel] [IOEMU][STUBDOM] build
fixes"):> Should this stuff be going in for Xen 4.0.0? It''s getting late and
I don''t
> think it''s a feature regression. If this patch is a
non-controversial
> obvious cleanup then that''s probably okay.
In general the intent is a cleanup and the parts of it that we can
agree on should go into 4.0.0. I''ll pick out the hunks that are
uncontroversial.
Unfortunately there are still some parts that need to be discussed
further.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel