[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH v4] os-android: Add support to android platf
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [RFC PATCH v4] os-android: Add support to android platform |
Date: |
Tue, 6 Oct 2015 10:47:13 +0100 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
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.