| Message ID | 1443847454-20406-1-git-send-email-houcheng@gmail.com |
|---|---|
| State | New |
| Headers | show |
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.
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
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).
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
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
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
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(-)