[Qemu-devel] [RFC PATCH v4] os-android: Add support to android platform

[Qemu-devel] [RFC PATCH v4] os-android: Add support to android platform

[Houcheng Lin]

Hi,

The v4 patch uses qemu_getdtablesize(), remove os-android.h and
use a local buffer when calling ptsname_r(). This patch is based
on version: 007e620a7576e4ce2ea6955541e87d8ae8ed32ae.

-----------------------------------------------------------------------

Building QEMU on android reqiures android NDK r10 cross-compilation
toolchain with following changes:

    - install qemu dependent libraries
    - upgrade bionic libc
    - add mssing includes, scsi/sg.h, in toolchain
    - make a symbolic link x86_64-linux-android-pkg-config that links
      to host's pkg-config.

Then, configure and build static linked qemu by commands:
  $ export SYSROOT="/opt/android-toolchain64/sysroot"
  $ PKG_CONFIG_LIBDIR=/opt/android-toolchain64/sysroot/usr/lib/pkgconfig \
    ./configure \
    --cross-prefix=x86_64-linux-android- --enable-kvm \
    --enable-trace-backend=nop --disable-fdt --target-list=x86_64-softmmu \
    --disable-spice --disable-vhost-net --disable-libiscsi \
    --audio-drv-list="" --disable-gtk --disable-gnutls \
    --disable-libnfs --disable-glusterfs --disable-libssh2 \
    --disable-seccomp --disable-usb-redir --disable-libusb \
    --disable-guest-agent --static
  $ make -j4

For dynamic build, you can skip the "static" option during configure, copy
dependent so files into android and add "." into LD_LIBRARY_PATH.

How to prepare your cross-compilation toolcahin
-----------------------------------------------

1. Download NDK r10, install toolchain from NDK and build the following libraries and install in your toolchain sysroot:
        libiconv-1.14
        gettext-0.19
        libffi-3.0.12
        glib-2.34.3
        libpng-1.2.52
        pixman-0.30

2. Download AOSP and apply this patch I made to support posix lockf()
    https://android-review.googlesource.com/#/c/172150/

3. Build bionic C and update your toolchain's libc.a and libc.so.

4. Copy kernel header file, scsi/sg.h into toolchain's sysroot/usr/includes/scsi/

5. Update these header files in your toolchain to prevent compilation warning,
   includes:
        unistd.h for lockf() and related define
        sys/uio.h for pread() and pwrite()
        signal.h for sigtimedwait()

Signed-off-by: Houcheng Lin <houcheng@gmail.com>
---
 configure                  | 14 ++++++++++++--
 default-configs/pci.mak    |  2 +-
 hw/i386/kvm/pci-assign.c   |  1 -
 hw/xenpv/xen_domainbuild.c |  2 +-
 include/qemu/osdep.h       | 10 ++++++++++
 kvm-all.c                  |  4 ++++
 slirp/misc.c               |  2 +-
 tests/Makefile             |  2 ++
 util/oslib-posix.c         | 16 ++++++++++++++++
 util/qemu-openpty.c        | 21 ++++++++++++++++-----
 10 files changed, 63 insertions(+), 11 deletions(-)

diff --git a/configure b/configure
index d7c24cd..cda88c1 100755
--- a/configure
+++ b/configure
@@ -567,7 +567,6 @@ fi
 
 # host *BSD for user mode
 HOST_VARIANT_DIR=""
-
 case $targetos in
 CYGWIN*)
   mingw32="yes"
@@ -693,6 +692,13 @@ Haiku)
   vhost_net="yes"
   vhost_scsi="yes"
   QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$(pwd)/linux-headers $QEMU_INCLUDES"
+  case $cross_prefix in
+    *android*)
+      android="yes"
+    ;;
+    *)
+    ;;
+  esac
 ;;
 esac
 
@@ -3791,7 +3797,7 @@ elif compile_prog "" "$pthread_lib -lrt" ; then
 fi
 
 if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a \
-        "$aix" != "yes" -a "$haiku" != "yes" ; then
+        "$aix" != "yes" -a "$haiku" != "yes" -a "$android" != "yes" ; then
     libs_softmmu="-lutil $libs_softmmu"
 fi
 
@@ -4737,6 +4743,10 @@ if test "$linux" = "yes" ; then
   echo "CONFIG_LINUX=y" >> $config_host_mak
 fi
 
+if test "$android" = "yes" ; then
+  echo "CONFIG_ANDROID=y" >> $config_host_mak
+fi
+
 if test "$darwin" = "yes" ; then
   echo "CONFIG_DARWIN=y" >> $config_host_mak
 fi
diff --git a/default-configs/pci.mak b/default-configs/pci.mak
index 7e10903..e76dd41 100644
--- a/default-configs/pci.mak
+++ b/default-configs/pci.mak
@@ -35,5 +35,5 @@ CONFIG_SDHCI=y
 CONFIG_EDU=y
 CONFIG_VGA=y
 CONFIG_VGA_PCI=y
-CONFIG_IVSHMEM=$(CONFIG_KVM)
+CONFIG_IVSHMEM=$(call land,$(call lnot,$(CONFIG_ANDROID)),$(CONFIG_KVM))
 CONFIG_ROCKER=y
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index b1beaa6..44beee3 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -22,7 +22,6 @@
  */
 #include <stdio.h>
 #include <unistd.h>
-#include <sys/io.h>
 #include <sys/mman.h>
 #include <sys/types.h>
 #include <sys/stat.h>
diff --git a/hw/xenpv/xen_domainbuild.c b/hw/xenpv/xen_domainbuild.c
index c0ab753..480ee87 100644
--- a/hw/xenpv/xen_domainbuild.c
+++ b/hw/xenpv/xen_domainbuild.c
@@ -170,7 +170,7 @@ static int xen_domain_watcher(void)
 
     /* close all file handles, except stdio/out/err,
      * our watch pipe and the xen interface handle */
-    n = getdtablesize();
+    n = qemu_getdtablesize();
     for (i = 3; i < n; i++) {
         if (i == fd[0])
             continue;
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index ab3c876..9e26d10 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -74,6 +74,14 @@ typedef unsigned int            uint_fast16_t;
 typedef signed int              int_fast16_t;
 #endif
 
+#ifdef CONFIG_ANDROID
+/*
+ * For include the basename prototyping in android.
+ */
+#include <libgen.h>
+#define IOV_MAX 1024
+#endif
+
 #ifndef O_LARGEFILE
 #define O_LARGEFILE 0
 #endif
@@ -284,4 +292,6 @@ void os_mem_prealloc(int fd, char *area, size_t sz);
 
 int qemu_read_password(char *buf, int buf_size);
 
+int qemu_getdtablesize(void);
+
 #endif
diff --git a/kvm-all.c b/kvm-all.c
index c6f5128..f7bb9c7 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -45,7 +45,11 @@
 #endif
 
 /* KVM uses PAGE_SIZE in its definition of COALESCED_MMIO_MAX */
+#ifdef PAGE_SIZE
+QEMU_BUILD_BUG_ON(PAGE_SIZE != TARGET_PAGE_SIZE);
+#else
 #define PAGE_SIZE TARGET_PAGE_SIZE
+#endif
 
 //#define DEBUG_KVM
 
diff --git a/slirp/misc.c b/slirp/misc.c
index 578e8b2..47dc71d 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -170,7 +170,7 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
 		dup2(s, 0);
 		dup2(s, 1);
 		dup2(s, 2);
-		for (s = getdtablesize() - 1; s >= 3; s--)
+		for (s = qemu_getdtablesize() - 1; s >= 3; s--)
 		   close(s);
 
 		i = 0;
diff --git a/tests/Makefile b/tests/Makefile
index 34c6136..99faf1f 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -418,8 +418,10 @@ tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a libqemustub.
 tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o $(block-obj-y) libqemuutil.a libqemustub.a
 
 ifeq ($(CONFIG_POSIX),y)
+ifneq ($(CONFIG_ANDROID),y)
 LIBS += -lutil
 endif
+endif
 
 # QTest rules
 
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 3ae4987..4ae746b 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -62,6 +62,8 @@ extern int daemon(int, int);
 #include <libgen.h>
 #include <setjmp.h>
 #include <sys/signal.h>
+#include <sys/time.h>
+#include <sys/resource.h>
 
 #ifdef CONFIG_LINUX
 #include <sys/syscall.h>
@@ -482,3 +484,17 @@ int qemu_read_password(char *buf, int buf_size)
     printf("\n");
     return ret;
 }
+
+int qemu_getdtablesize(void)
+{
+#ifdef CONFIG_ANDROID
+    struct rlimit r;
+
+    if (getrlimit(RLIMIT_NOFILE, &r) < 0) {
+        return sysconf(_SC_OPEN_MAX);
+    }
+    return r.rlim_cur;
+#else
+    return getdtablesize();
+#endif
+}
diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c
index 4c53211..b305886 100644
--- a/util/qemu-openpty.c
+++ b/util/qemu-openpty.c
@@ -51,12 +51,17 @@
 # include <termios.h>
 #endif
 
-#ifdef __sun__
+#if defined(__sun__) || defined(CONFIG_ANDROID)
+
 /* Once Solaris has openpty(), this is going to be removed. */
 static int openpty(int *amaster, int *aslave, char *name,
                    struct termios *termp, struct winsize *winp)
 {
+#if defined(CONFIG_ANDROID)
+        char slave[PATH_MAX];
+#else
         const char *slave;
+#endif
         int mfd = -1, sfd = -1;
 
         *amaster = *aslave = -1;
@@ -67,17 +72,22 @@ static int openpty(int *amaster, int *aslave, char *name,
 
         if (grantpt(mfd) == -1 || unlockpt(mfd) == -1)
                 goto err;
-
+#if defined(CONFIG_ANDROID)
+        if (ptsname_r(mfd, slave, PATH_MAX) < 0)
+                goto err;
+#else
         if ((slave = ptsname(mfd)) == NULL)
                 goto err;
+#endif
 
         if ((sfd = open(slave, O_RDONLY | O_NOCTTY)) == -1)
                 goto err;
 
+#ifndef CONFIG_ANDROID
         if (ioctl(sfd, I_PUSH, "ptem") == -1 ||
             (termp != NULL && tcgetattr(sfd, termp) < 0))
                 goto err;
-
+#endif
         if (amaster)
                 *amaster = mfd;
         if (aslave)
@@ -93,7 +103,8 @@ err:
         close(mfd);
         return -1;
 }
-
+#endif
+#ifdef __sun__
 static void cfmakeraw (struct termios *termios_p)
 {
         termios_p->c_iflag &=
@@ -112,7 +123,7 @@ int qemu_openpty_raw(int *aslave, char *pty_name)
 {
     int amaster;
     struct termios tty;
-#if defined(__OpenBSD__) || defined(__DragonFly__)
+#if defined(__OpenBSD__) || defined(__DragonFly__) || defined(CONFIG_ANDROID)
     char pty_buf[PATH_MAX];
 #define q_ptsname(x) pty_buf
 #else
-- 
1.9.1

Re: [Qemu-devel] [RFC PATCH v4] os-android: Add support to android platform

[Stefan Hajnoczi]

On Sat, Oct 03, 2015 at 12:44:14PM +0800, Houcheng Lin wrote:
> diff --git a/configure b/configure
> index d7c24cd..cda88c1 100755
> --- a/configure
> +++ b/configure
> @@ -567,7 +567,6 @@ fi
>  
>  # host *BSD for user mode
>  HOST_VARIANT_DIR=""
> -
>  case $targetos in
>  CYGWIN*)
>    mingw32="yes"

Spurious whitespace change

> diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
> index b1beaa6..44beee3 100644
> --- a/hw/i386/kvm/pci-assign.c
> +++ b/hw/i386/kvm/pci-assign.c
> @@ -22,7 +22,6 @@
>   */
>  #include <stdio.h>
>  #include <unistd.h>
> -#include <sys/io.h>
>  #include <sys/mman.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>

What is the justification for this?  Do you know why io.h was included
before?

> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index ab3c876..9e26d10 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -74,6 +74,14 @@ typedef unsigned int            uint_fast16_t;
>  typedef signed int              int_fast16_t;
>  #endif
>  
> +#ifdef CONFIG_ANDROID
> +/*
> + * For include the basename prototyping in android.
> + */
> +#include <libgen.h>

Files that use basename(3) should include libgen.h.  Why include it
here?

> +#define IOV_MAX 1024

Are you sure that Android NDK headers do not contain this constant?

> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 3ae4987..4ae746b 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -62,6 +62,8 @@ extern int daemon(int, int);
>  #include <libgen.h>
>  #include <setjmp.h>
>  #include <sys/signal.h>
> +#include <sys/time.h>

Why did you include time.h?

> +#include <sys/resource.h>
>  
>  #ifdef CONFIG_LINUX
>  #include <sys/syscall.h>
> @@ -482,3 +484,17 @@ int qemu_read_password(char *buf, int buf_size)
>      printf("\n");
>      return ret;
>  }
> +
> +int qemu_getdtablesize(void)
> +{
> +#ifdef CONFIG_ANDROID
> +    struct rlimit r;
> +
> +    if (getrlimit(RLIMIT_NOFILE, &r) < 0) {
> +        return sysconf(_SC_OPEN_MAX);
> +    }
> +    return r.rlim_cur;
> +#else
> +    return getdtablesize();
> +#endif
> +}

We can probably drop the getdtablesize() call completely and use the
CONFIG_ANDROID code on all platforms.  I suggest splitting this out into
a separate patch that introduces qemu_getdtablesize() and converts all
callers.

> diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c
> index 4c53211..b305886 100644
> --- a/util/qemu-openpty.c
> +++ b/util/qemu-openpty.c
> @@ -51,12 +51,17 @@
>  # include <termios.h>
>  #endif
>  
> -#ifdef __sun__
> +#if defined(__sun__) || defined(CONFIG_ANDROID)
> +
>  /* Once Solaris has openpty(), this is going to be removed. */
>  static int openpty(int *amaster, int *aslave, char *name,
>                     struct termios *termp, struct winsize *winp)
>  {
> +#if defined(CONFIG_ANDROID)
> +        char slave[PATH_MAX];
> +#else
>          const char *slave;
> +#endif
>          int mfd = -1, sfd = -1;
>  
>          *amaster = *aslave = -1;
> @@ -67,17 +72,22 @@ static int openpty(int *amaster, int *aslave, char *name,
>  
>          if (grantpt(mfd) == -1 || unlockpt(mfd) == -1)
>                  goto err;
> -
> +#if defined(CONFIG_ANDROID)
> +        if (ptsname_r(mfd, slave, PATH_MAX) < 0)
> +                goto err;
> +#else
>          if ((slave = ptsname(mfd)) == NULL)
>                  goto err;
> +#endif

ptsname_r(3) should be used on all Linux hosts because it is reentrant.
This improvement isn't Android-specific, please split it into a separate
patch.

Re: [Qemu-devel] [RFC PATCH v4] os-android: Add support to android platform

[Paolo Bonzini]

Just a couple comments since I reviewed the previous versions...

On 06/10/2015 11:47, Stefan Hajnoczi wrote:
> >  #include <unistd.h>
> > -#include <sys/io.h>
> >  #include <sys/mman.h>
> >  #include <sys/types.h>
> >  #include <sys/stat.h>
> 
> What is the justification for this?  Do you know why io.h was included
> before?

No reason, the same patch is en route through qemu-trivial.

>>
>> -
>> +#if defined(CONFIG_ANDROID)
>> +        if (ptsname_r(mfd, slave, PATH_MAX) < 0)
>> +                goto err;
>> +#else
>>          if ((slave = ptsname(mfd)) == NULL)
>>                  goto err;
>> +#endif
> 
> ptsname_r(3) should be used on all Linux hosts because it is reentrant.
> This improvement isn't Android-specific, please split it into a separate
> patch.

Actually everyone except Solaris and Android is already using openpty.
This is emulation code for those two OSes.  (The gnulib manual mentions
that AIX 5.1, HP-UX 11, IRIX 6.5 also don't have openpty, but we don't
support those I think).

Paolo

Re: [Qemu-devel] [RFC PATCH v4] os-android: Add support to android platform

[Eric Blake]

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

On 10/06/2015 03:47 AM, Stefan Hajnoczi wrote:
> On Sat, Oct 03, 2015 at 12:44:14PM +0800, Houcheng Lin wrote:
>> diff --git a/configure b/configure
>> index d7c24cd..cda88c1 100755

>> +++ b/include/qemu/osdep.h
>> @@ -74,6 +74,14 @@ typedef unsigned int            uint_fast16_t;
>>  typedef signed int              int_fast16_t;
>>  #endif
>>  
>> +#ifdef CONFIG_ANDROID
>> +/*
>> + * For include the basename prototyping in android.
>> + */
>> +#include <libgen.h>
> 
> Files that use basename(3) should include libgen.h.  Why include it
> here?

What is using basename(3)? POSIX gives basename(3) a worthless contract
- it is free to return non-thread-safe storage.  Also, it is not
portable to Windows-style drive-letters, so I consider any code using
<libgen.h> to already be suspect.

If you are already writing code to be ported to both Unixy and windows
systems, you are better off rolling your own alternative to basename (or
better, using something else that has already rolled a portable version
for you - while I know gnulib does that, we aren't using gnulib; but I
assume glib has something along those lines even though I haven't looked
for it).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [RFC PATCH v4] os-android: Add support to android platform

[Paolo Bonzini]

On 06/10/2015 14:13, Eric Blake wrote:
> 
> If you are already writing code to be ported to both Unixy and
> windows systems, you are better off rolling your own alternative to
> basename (or better, using something else that has already rolled a
> portable version for you - while I know gnulib does that, we aren't
> using gnulib; but I assume glib has something along those lines
> even though I haven't looked for it).

Yes, there is g_path_get_basename (and g_path_get_dirname).  Added to
http://wiki.qemu.org/BiteSizedTasks#API_conversion.

Paolo

Re: [Qemu-devel] [RFC PATCH v4] os-android: Add support to android platform

[Houcheng Lin]

Hi,

There are 7 sources still call basename() directly and block/vvfat.c
define its own static basename() function. Please see the grep below:

➜  qemu git:(patch-v4) ✗ grep  "basename(" **/*.c  | grep -v get_basename
fsdev/virtfs-proxy-helper.c:            basename(prog));
hw/vfio/pci.c:    group_name = basename(iommu_group_path);
hw/vfio/platform.c:    group_name = basename(iommu_group_path);
linux-user/elfload.c:    base_filename = strdup(basename(filename));
qemu-io.c:    progname = basename(argv[0]);
qemu-nbd.c:        snprintf(sockpath, 128, SOCKET_PATH, basename(device));
qga/commands-posix.c:        driver = g_strdup(basename(buf));
qga/commands-posix.c:        fs->name = g_strdup(basename(syspath));

➜  qemu git:(patch-v4) ✗ grep get_basename **/*.c
block/vvfat.c:static const char* get_basename(const char* path)
block/vvfat.c: basename2 = get_basename(path);
block/vvfat.c:    basename = get_basename(mapping->path);
block/vvfat.c: const char* basename = get_basename(mapping->path);
block/vvfat.c: const char* basename = get_basename(mapping->path);
block/vvfat.c: const char* basename2 = get_basename(path);
block/vvfat.c: - strlen(get_basename(commit->path)) - 1;
linux-user/elfload.c:    base_filename =
g_path_get_basename(ts->bprm->filename);

Directly change all of them to g_path_get_basename ?

2015-10-06 21:22 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>
>
> On 06/10/2015 14:13, Eric Blake wrote:
>>
>> If you are already writing code to be ported to both Unixy and
>> windows systems, you are better off rolling your own alternative to
>> basename (or better, using something else that has already rolled a
>> portable version for you - while I know gnulib does that, we aren't
>> using gnulib; but I assume glib has something along those lines
>> even though I haven't looked for it).
>
> Yes, there is g_path_get_basename (and g_path_get_dirname).  Added to
> http://wiki.qemu.org/BiteSizedTasks#API_conversion.
>
> Paolo



-- 
Best regards,
Houcheng Lin