[Qemu-devel] FreeBSD Support

[Qemu-devel] FreeBSD Support

[Christopher Olsen]

This is a sidetrack here... But is it at all possible to make future releases 
of the source more FreeBSD friendly?

I am willing to work off a specified codebase to bring it up to FreeBSD speed 
so it can be easily maintained from there... 

Because unfortunately out of the box it doesn't build.

-Christopher

-- 
Christopher Olsen
cwolsen@domainatlantic.com
Tel: 631-676-4877
Fax: 631-249-3036

Re: [Qemu-devel] FreeBSD Support

[Paul Brook]

> This is a sidetrack here... But is it at all possible to make future
> releases of the source more FreeBSD friendly?

If someone puts in the effort to make it so, yes.

Note that dumping the current patches from FreeBSD ports on the list is 
generally not sufficient. Blindly posting patches without explanation is a 
good way to get them ignored.

All patches should include a description of what they are fixing (ie. what 
qemu currently does wrong), how the patch fixes it, and why this is the right 
way to fix it. For OS specific hacks there needs to be a good reason why that 
OS is special. 
If you can't answer all the above questions then you shouldn't be submitting 
the patch. In some cases you may need to rewriting the patches to meet these 
criteria. Individual OS/Distros tend to be less picky because they only care 
whether the result works on that one system.

Paul

Re: [Qemu-devel] FreeBSD Support

[Ben Taylor]

---- Christopher Olsen <cwolsen@domainatlantic.com> wrote: 
> This is a sidetrack here... But is it at all possible to make future releases 
> of the source more FreeBSD friendly?
> 
> I am willing to work off a specified codebase to bring it up to FreeBSD speed 
> so it can be easily maintained from there... 
> 
> Because unfortunately out of the box it doesn't build.

Having been in your shoes, the only thing I can tell you is start
small.  Be able to justify your patches and change as little as
possible.  Be smart about how you code things if there are 
specific reasons.  And get ready for a long haul.  Be ready to
constantly be porting your changes through the CVS tree.

I can tell you that you won't be able to submit a 

"This patch makes it work with FreeBSD"

and have it accepted.

Start with the configure script and Makefiles, and any very specfic, targeted
and small patches  and let those changes slowly propogate out.

HTH,

Ben

Re: [Qemu-devel] FreeBSD Support

[Christopher Olsen]

On Monday 19 February 2007 20:46, Paul Brook wrote:
> > This is a sidetrack here... But is it at all possible to make future
> > releases of the source more FreeBSD friendly?
>
> If someone puts in the effort to make it so, yes.
>
> Note that dumping the current patches from FreeBSD ports on the list is
> generally not sufficient. Blindly posting patches without explanation is a
> good way to get them ignored.
>
> All patches should include a description of what they are fixing (ie. what
> qemu currently does wrong), how the patch fixes it, and why this is the
> right way to fix it. For OS specific hacks there needs to be a good reason
> why that OS is special.
> If you can't answer all the above questions then you shouldn't be
> submitting the patch. In some cases you may need to rewriting the patches
> to meet these criteria. Individual OS/Distros tend to be less picky because
> they only care whether the result works on that one system.
>
> Paul
>
>
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel

Paul,

So should I make a separate patch for each modified file?

-Christopher

-- 
Christopher Olsen
cwolsen@domainatlantic.com
Tel: 631-676-4877
Fax: 631-249-3036

Re: [Qemu-devel] FreeBSD Support

[Christopher Olsen]

On Monday 19 February 2007 21:08, Ben Taylor wrote:
> Having been in your shoes, the only thing I can tell you is start
> small.  Be able to justify your patches and change as little as
> possible.  Be smart about how you code things if there are
> specific reasons.  And get ready for a long haul.  Be ready to
> constantly be porting your changes through the CVS tree.
>
> I can tell you that you won't be able to submit a
>
> "This patch makes it work with FreeBSD"
>
> and have it accepted.
>
> Start with the configure script and Makefiles, and any very specfic,
> targeted and small patches  and let those changes slowly propogate out.
>
> HTH,
>
> Ben
>
>
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel

Well that shouldn't be an issue... Plus I have particular interest in this 
project. That and I've stuck by QEMU because of it's significant performance 
over BOCHS as the PC emulator of choice when I developed my operating system

-Christopher

-- 
Christopher Olsen
cwolsen@domainatlantic.com
Tel: 631-676-4877
Fax: 631-249-3036

Re: [Qemu-devel] FreeBSD Support

[Paul Brook]

> So should I make a separate patch for each modified file?

No. You should break changes into logically independent patches.
It is ok for a single patch to touch multiple files, but it should only fix 
one thing.

Paul

[Qemu-devel] FreeBSD Support

[Christopher Olsen]

[-- Attachment #1: Type: text/plain, Size: 758 bytes --]

Ok FreeBSD Support round one..

Be gentle this is my first attempt at working with the rest of this 
community..

Files it modifies and the reasons are as follows

configure - Adds HOST_FREEBSD type to alter included libraries FreeBSD does 
not need -ltr 
Makefile.target - Once again uses HOST_FREEBSD to avoid including -ltr

osdeps.c - FreeBSD does not have /dev/shm so it uses /tmp for kqemu_valloc 
also sys/vfs.h is not part of freebsd stat information is part of 
mount.h/param.h

vl.c - FreeBSD needs module if_tap for tap not default in kernel and fopen the 
tap device with a workaround for FreeBSD related issues for incrementing the 
tap device

-Christopher


-- 
Christopher Olsen
cwolsen@domainatlantic.com
Tel: 631-676-4877
Fax: 631-249-3036

[-- Attachment #2: qemu.freebsd.patch --]
[-- Type: text/x-diff, Size: 4093 bytes --]

--- configure	Mon Feb 19 21:49:15 2007
+++ /home/reddawg/qemu-0.9.0/configure	Mon Feb 19 21:00:11 2007
@@ -111,6 +111,7 @@
 ;;
 FreeBSD)
 bsd="yes"
+freebsd="yes"
 oss="yes"
 if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then
     kqemu="yes"
@@ -719,6 +720,10 @@
 if test "$solaris" = "yes" ; then
   echo "CONFIG_SOLARIS=yes" >> $config_mak
   echo "#define HOST_SOLARIS $solarisrev" >> $config_h
+fi
+if test "$freebsd" = "yes" ; then
+  echo "CONFIG_FREEBSD=yes" >> $config_mak
+  echo "#define HOST_FREEBSD 1" >> $config_h
 fi
 if test "$gdbstub" = "yes" ; then
   echo "CONFIG_GDBSTUB=yes" >> $config_mak
--- Makefile.target	Mon Feb 19 21:49:15 2007
+++ /home/reddawg/qemu-0.9.0/Makefile.target	Mon Feb 19 21:07:04 2007
@@ -440,7 +440,11 @@
 ifndef CONFIG_DARWIN
 ifndef CONFIG_WIN32
 ifndef CONFIG_SOLARIS
+ifndef CONFIG_FREEBSD
 VL_LIBS=-lutil -lrt
+else
+VL_LIBS=-lutil
+endif
 endif
 endif
 endif
--- osdep.c	Mon Feb 19 22:01:46 2007
+++ /home/reddawg/qemu-0.9.0/osdep.c	Mon Feb 19 21:40:45 2007
@@ -79,7 +79,12 @@
 
 #if defined(USE_KQEMU)
 
+#ifdef HOST_FREEBSD
+#include <sys/param.h>
+#include <sys/mount.h>
+#else
 #include <sys/vfs.h>
+#endif
 #include <sys/mman.h>
 #include <fcntl.h>
 
@@ -102,6 +107,9 @@
 #ifdef HOST_SOLARIS
             tmpdir = "/tmp";
         if (statvfs(tmpdir, &stfs) == 0) {
+#elif HOST_FREEBSD
+            tmpdir = "/tmp";
+        if (statfs(tmpdir, &stfs) == 0) {
 #else
             tmpdir = "/dev/shm";
         if (statfs(tmpdir, &stfs) == 0) {
--- vl.c	Mon Feb 19 22:01:46 2007
+++ /home/reddawg/qemu-0.9.0/vl.c	Mon Feb 19 22:10:06 2007
@@ -47,6 +47,9 @@
 #ifndef __APPLE__
 #include <libutil.h>
 #endif
+#ifdef HOST_FREEBSD 
+#include <sys/module.h>
+#endif
 #else
 #ifndef __sun__
 #include <linux/if.h>
@@ -3171,6 +3174,34 @@
 
 #endif /* CONFIG_SLIRP */
 
+#ifdef HOST_FREEBSD
+
+#define LOAD_QUIETLY   1
+#define LOAD_VERBOSLY  2
+
+/* This function is used to load needed kernel modules for tap */
+int loadmodules(int how, const char *module, ...) {
+  int loaded = 0;
+  va_list ap;
+
+  va_start(ap, module);
+#ifndef NO_MODULES
+  while (module != NULL) {
+    if (modfind(module) == -1) {
+      if (kldload(module) == -1) {
+        if (how == LOAD_VERBOSLY)
+          fprintf(stderr, "%s: Cannot load module\n", module);
+      } else
+        loaded++;
+    }
+    module = va_arg(ap, const char *);
+  }
+  va_end(ap);
+#endif
+  return loaded;
+}
+#endif
+
 #if !defined(_WIN32)
 
 typedef struct TAPState {
@@ -3226,11 +3257,55 @@
     char *dev;
     struct stat s;
 
+#ifdef HOST_FREEBSD
+    int i, kldtried = 0, enoentcount = 0, err = 0;
+    char dname[100];
+#ifdef USE_DEVTAP
+    /*
+     * 5.x has /dev/tap, but that seems to just blindly increase its
+     * couter on every open() for some people(??), i.e. on every qemu run.
+     */
+    i = -1;
+#else
+    i = 0;
+#endif
+    for (; i < 10; i++) {
+        if (i == -1)
+           strcpy(dname, "/dev/tap");
+        else
+           snprintf(dname, sizeof dname, "%s%d",
+                    "/dev/tap", i);
+        fd = open(dname, O_RDWR);
+        if (fd >= 0)
+            break;
+        else if (errno == ENXIO || errno == ENOENT) {
+            if (i == 0 && !kldtried++) {
+                /*
+                 * Attempt to load the tunnel interface KLD if it isn't loaded
+                 * already.
+                 */
+                if (loadmodules(LOAD_VERBOSLY, "if_tap", NULL))
+                    i = -1;
+                continue;
+            }
+            if (errno != ENOENT || ++enoentcount > 3) {
+                err = errno;
+               break;
+            }
+        } else
+            err = errno;
+    }
+    if (fd < 0) {
+        fprintf(stderr, "warning: could not open %s (%s): no virtual network emulation\n", dname, strerror(err));
+        return -1;
+    }
+#else
     fd = open("/dev/tap", O_RDWR);
     if (fd < 0) {
         fprintf(stderr, "warning: could not open /dev/tap: no virtual network emulation\n");
         return -1;
     }
+#endif
 
     fstat(fd, &s);
     dev = devname(s.st_rdev, S_IFCHR);

Re: [Qemu-devel] FreeBSD Support

[M. Warner Losh]

In message: <20894890.1171937332995.JavaMail.root@eastrmwml08.mgt.cox.net>
            Ben Taylor <sol10x86@cox.net> writes:
: Start with the configure script and Makefiles, and any very specfic, targeted
: and small patches  and let those changes slowly propogate out.

Most of the FreeBSD ports patches are relatively easy to justify and
explain.  For example, FreeBSD port system defines SDL_CONFIG in a way
that's overridable.  There's a patch that looks like:

Index: qemu/configure
@@ -204,7 +204,7 @@
 
 if test -z "$sdl" ; then
 
-sdl_config="sdl-config"
+sdl_config="${SDL_CONFIG}"
 sdl=no
 sdl_static=no
 
which is straight forward enought, but still wrong...  It should be:

Index: qemu/configure
@@ -204,7 +204,7 @@
 
 if test -z "$sdl" ; then
 
-sdl_config="sdl-config"
+sdl_config="${SDL_CONFIG:-sdl-config}"
 sdl=no
 sdl_static=no
 
to still work on non-FreeBSD systems or on FreeBSD systems not using
the ports....

There's a lot of niggles like this, however...

Warner

Re: [Qemu-devel] FreeBSD Support

[Juergen Lock]

In article <200702192226.45410.cwolsen@domainatlantic.com> you write:
>-=-=-=-=-=-
>
>Ok FreeBSD Support round one..
>
>Be gentle this is my first attempt at working with the rest of this 
>community..
>
>Files it modifies and the reasons are as follows
>
>configure - Adds HOST_FREEBSD type to alter included libraries FreeBSD does 
>not need -ltr 
>Makefile.target - Once again uses HOST_FREEBSD to avoid including -ltr
>
>osdeps.c - FreeBSD does not have /dev/shm so it uses /tmp for kqemu_valloc 
>also sys/vfs.h is not part of freebsd stat information is part of 
>mount.h/param.h
>...

Actually the port simply uses mmap MAP_PRIVATE|MAP_ANON, so no
tempfile is used at all (this is files/patch-osdep.c in the port):

Index: qemu/osdep.c
@@ -79,7 +79,9 @@
 
 #if defined(USE_KQEMU)
 
+#ifndef __FreeBSD__
 #include <sys/vfs.h>
+#endif
 #include <sys/mman.h>
 #include <fcntl.h>
 
@@ -90,6 +92,7 @@
     const char *tmpdir;
     char phys_ram_file[1024];
     void *ptr;
+#ifndef __FreeBSD__
 #ifdef HOST_SOLARIS
     struct statvfs stfs;
 #else
@@ -151,12 +154,20 @@
         }
         unlink(phys_ram_file);
     }
+#endif
     size = (size + 4095) & ~4095;
+#ifndef __FreeBSD__
     ftruncate(phys_ram_fd, phys_ram_size + size);
     ptr = mmap(NULL, 
                size, 
                PROT_WRITE | PROT_READ, MAP_SHARED, 
                phys_ram_fd, phys_ram_size);
+#else
+    ptr = mmap(NULL, 
+               size, 
+               PROT_WRITE | PROT_READ, MAP_PRIVATE|MAP_ANON, 
+               -1, 0);
+#endif
     if (ptr == MAP_FAILED) {
         fprintf(stderr, "Could not map physical memory\n");
         exit(1);

Re: [Qemu-devel] FreeBSD Support

[Leonardo Reiter]

MAP_PRIVATE|MAP_ANON also works on Solaris.  In fact, Linux is the
only platform where it doesn't work due to a bug in the Linux kernel
as Fabrice mentions:

http://www.qemu.org/kqemu-tech.html#SEC7

Technically on Solaris, /tmp is probably the same thing as
MAP_PRIVATE|MAP_ANON since the filesystem is actually mapped to
virtual memory... however, it's much cleaner to not use a file if not
necessary.  I can post a reworked patch that makes the file mapping
for the Linux case only if anyone's interested, but it's a pretty
simple change.

- Leo Reiter

On 2/24/07, Juergen Lock <nox@jelal.kn-bremen.de> wrote:
> In article <200702192226.45410.cwolsen@domainatlantic.com> you write:
> >-=-=-=-=-=-
> >
> >Ok FreeBSD Support round one..
> >
> >Be gentle this is my first attempt at working with the rest of this
> >community..
> >
> >Files it modifies and the reasons are as follows
> >
> >configure - Adds HOST_FREEBSD type to alter included libraries FreeBSD does
> >not need -ltr
> >Makefile.target - Once again uses HOST_FREEBSD to avoid including -ltr
> >
> >osdeps.c - FreeBSD does not have /dev/shm so it uses /tmp for kqemu_valloc
> >also sys/vfs.h is not part of freebsd stat information is part of
> >mount.h/param.h
> >...
>
> Actually the port simply uses mmap MAP_PRIVATE|MAP_ANON, so no
> tempfile is used at all (this is files/patch-osdep.c in the port):
>
> Index: qemu/osdep.c
> @@ -79,7 +79,9 @@
>
>  #if defined(USE_KQEMU)
>
> +#ifndef __FreeBSD__
>  #include <sys/vfs.h>
> +#endif
>  #include <sys/mman.h>
>  #include <fcntl.h>
>
> @@ -90,6 +92,7 @@
>      const char *tmpdir;
>      char phys_ram_file[1024];
>      void *ptr;
> +#ifndef __FreeBSD__
>  #ifdef HOST_SOLARIS
>      struct statvfs stfs;
>  #else
> @@ -151,12 +154,20 @@
>          }
>          unlink(phys_ram_file);
>      }
> +#endif
>      size = (size + 4095) & ~4095;
> +#ifndef __FreeBSD__
>      ftruncate(phys_ram_fd, phys_ram_size + size);
>      ptr = mmap(NULL,
>                 size,
>                 PROT_WRITE | PROT_READ, MAP_SHARED,
>                 phys_ram_fd, phys_ram_size);
> +#else
> +    ptr = mmap(NULL,
> +               size,
> +               PROT_WRITE | PROT_READ, MAP_PRIVATE|MAP_ANON,
> +               -1, 0);
> +#endif
>      if (ptr == MAP_FAILED) {
>          fprintf(stderr, "Could not map physical memory\n");
>          exit(1);
>
>
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel
>

Re: [Qemu-devel] FreeBSD Support

[andrzej zaborowski]

Hi,

On 20/02/07, Christopher Olsen <cwolsen@domainatlantic.com> wrote:
> Ok FreeBSD Support round one..
>
> Be gentle this is my first attempt at working with the rest of this
> community..
>
> Files it modifies and the reasons are as follows
>
> configure - Adds HOST_FREEBSD type to alter included libraries FreeBSD does
> not need -ltr
> Makefile.target - Once again uses HOST_FREEBSD to avoid including -ltr

See http://lists.gnu.org/archive/html/qemu-devel/2006-11/msg00166.html
- I believe this is a more appropriate way to select -lrt.

Regards,
Andrew

Re: [Qemu-devel] FreeBSD Support

[Thiemo Seufer]

andrzej zaborowski wrote:
> Hi,
> 
> On 20/02/07, Christopher Olsen <cwolsen@domainatlantic.com> wrote:
> >Ok FreeBSD Support round one..
> >
> >Be gentle this is my first attempt at working with the rest of this
> >community..
> >
> >Files it modifies and the reasons are as follows
> >
> >configure - Adds HOST_FREEBSD type to alter included libraries FreeBSD does
> >not need -ltr
> >Makefile.target - Once again uses HOST_FREEBSD to avoid including -ltr
> 
> See http://lists.gnu.org/archive/html/qemu-devel/2006-11/msg00166.html
> - I believe this is a more appropriate way to select -lrt.

The solaris part there looked a bit inconsistent, is the appended
version ok?


Thiemo


Index: qemu-work/Makefile
===================================================================
--- qemu-work.orig/Makefile	2007-02-11 16:10:55.000000000 +0000
+++ qemu-work/Makefile	2007-03-02 21:07:26.000000000 +0000
@@ -24,11 +24,7 @@
 DOCS=
 endif
 
-ifndef CONFIG_DARWIN
-ifndef CONFIG_WIN32
-LIBS+=-lrt
-endif
-endif
+LIBS+=$(AIOLIBS)
 
 all: $(TOOLS) $(DOCS) recurse-all
 
Index: qemu-work/Makefile.target
===================================================================
--- qemu-work.orig/Makefile.target	2007-03-02 20:57:01.000000000 +0000
+++ qemu-work/Makefile.target	2007-03-02 21:01:22.000000000 +0000
@@ -436,6 +436,7 @@
 endif
 
 VL_LDFLAGS=
+VL_LIBS=$(AIOLIBS)
 # specific flags are needed for non soft mmu emulator
 ifdef CONFIG_STATIC
 VL_LDFLAGS+=-static
@@ -446,7 +447,7 @@
 ifndef CONFIG_DARWIN
 ifndef CONFIG_WIN32
 ifndef CONFIG_SOLARIS
-VL_LIBS=-lutil -lrt
+VL_LIBS+=-lutil
 endif
 endif
 endif
Index: qemu-work/configure
===================================================================
--- qemu-work.orig/configure	2007-03-02 20:57:00.000000000 +0000
+++ qemu-work/configure	2007-03-02 21:07:10.000000000 +0000
@@ -159,6 +159,12 @@
   fi
 fi
 
+if [ "$bsd" = "yes" -o "$darwin" = "yes" -o "$solaris" = "yes" -o "$mingw32" = "yes" ] ; then
+    AIOLIBS=
+else
+    AIOLIBS="-lrt"
+fi
+
 # find source path
 source_path=`dirname "$0"`
 if [ -z "$source_path" ]; then
@@ -653,6 +659,7 @@
 echo "CFLAGS=$CFLAGS" >> $config_mak
 echo "LDFLAGS=$LDFLAGS" >> $config_mak
 echo "EXESUF=$EXESUF" >> $config_mak
+echo "AIOLIBS=$AIOLIBS" >> $config_mak
 if test "$cpu" = "i386" ; then
   echo "ARCH=i386" >> $config_mak
   echo "#define HOST_I386 1" >> $config_h

Re: [Qemu-devel] FreeBSD Support

[andrzej zaborowski]

Hi,

On 03/03/07, Thiemo Seufer <ths@networkno.de> wrote:
> andrzej zaborowski wrote:
> > Hi,
> >
> > On 20/02/07, Christopher Olsen <cwolsen@domainatlantic.com> wrote:
> > >Ok FreeBSD Support round one..
> > >
> > >Be gentle this is my first attempt at working with the rest of this
> > >community..
> > >
> > >Files it modifies and the reasons are as follows
> > >
> > >configure - Adds HOST_FREEBSD type to alter included libraries FreeBSD does
> > >not need -ltr
> > >Makefile.target - Once again uses HOST_FREEBSD to avoid including -ltr
> >
> > See http://lists.gnu.org/archive/html/qemu-devel/2006-11/msg00166.html
> > - I believe this is a more appropriate way to select -lrt.
>
> The solaris part there looked a bit inconsistent, is the appended
> version ok?

It works for FreeBSD, I think it's Ok.

Regards,
Andrew